Clean Code - Speaker Deck

Slide 1

Slide 1 text

Clean Code Benedikus H Tjuatja David Setyanugraha Fakhri Lutvianes Advendianto

Slide 2

Slide 2 text

No content

Slide 3

Slide 3 text

3 Overview ● Issues ● Solution ● Conclusion © GDP Labs 2017

Slide 4

Slide 4 text

4 Issues ● Bad Code ● Great vs Bad Company ● Who is Responsible? ! © GDP Labs 2017

Slide 5

Slide 5 text

5 ● Technical debt: the eventual consequences of poor software architecture and software development within a code base. ○ Rushed the product to the market ○ Had made a huge mess in the code ○ Added more features ○ The code got worse and worse Bad Code © GDP Labs 2017

Slide 6

Slide 6 text

6 © GDP Labs 2017 ● LeBlanc’s Law: Later equals never ○ They never go back to fix the bad code ● Sink projects, careers and companies ● Example: netscape and myspace Bad Code

Slide 7

Slide 7 text

7 Death Spiral: The downward, corkscrew-motion of a disabled aircraft which is unrecoverably headed for a crash

Slide 8

Slide 8 text

8 Great Company vs Bad Company © GDP Labs 2017

Slide 9

Slide 9 text

9 © GDP Labs 2017 Great Company vs Bad Company

Slide 10

Slide 10 text

10 © GDP Labs 2017 Great Company vs Bad Company

Slide 11

Slide 11 text

11 ● BOD? ● Management? ● Program Manager? ● Product Manager? ● Unrealistic schedule? Who is Responsible for Bad Code? Unprofessional Software Engineers. It is your job to defend your code with equal passion. © GDP Labs 2017

Slide 12

Slide 12 text

12 It is a crime if a surgeon does not “clean” his hands

Slide 13

Slide 13 text

For the things we have to learn before we can do them, we learn by doing them - Aristotle 13

Slide 14

Slide 14 text

14 Solution ● Naming ● Function ● Comment ● Formatting ● Objects and Data Structures ● Error Handling ● Class ● Emergence © GDP Labs 2017

Slide 15

Slide 15 text

15 Naming The name of a variable, function, or class, should answer all the big questions. It should tell you why it exists, what it does, and how it is used. - Robert C. Martin ” “ © GDP Labs 2017

Slide 16

Slide 16 text

16 public List getThem() { List list1 = new ArrayList(); for (int[] x : theList) if (x[0] == 4) list1.add(x); return list1; } Say that we’re working in a mine sweeper game. Naming © GDP Labs 2017

Slide 17

Slide 17 text

17 Use intention-revealing names public List getFlaggedCells() { List flaggedCells = new ArrayList(); for (int[] cell : gameBoard) if (cell[STATUS_VALUE] == FLAGGED) flaggedCells.add(cell); return flaggedCells; } Naming © GDP Labs 2017

Slide 18

Slide 18 text

18 Use intention-revealing names public List getFlaggedCells() { List flaggedCells = new ArrayList(); for (Cell cell : gameBoard) if (cell.isFlagged()) flaggedCells.add(cell); return flaggedCells; } Naming © GDP Labs 2017

Slide 19

Slide 19 text

19 Avoid misleading and unmeaningful distinctions int a = l; if (O == l) a = O1; else l = 01; private void copyChars(char a1[], char a2[]) { for (int i = 0; i < a1.length; i++) { a2[i] = a1[i]; } } Naming © GDP Labs 2017 MISLEADING UNMEANINGFUL DISTINCTIONS

Slide 20

Slide 20 text

20 public class DtaRcrd102 { private Date genymdhms; private Date modymdhms; private final String pszqint = ”102”; /* … */ } CAN YOU PRONOUNCE THEM? Naming © GDP Labs 2017

Slide 21

Slide 21 text

21 Use pronounceable names public class Customer { private Date generationTimestamp; private Date modificationTimestamp; private final String recordId = ”102”; /* … */ } Naming © GDP Labs 2017

Slide 22

Slide 22 text

22 Classes and objects should be noun, methods should be verb public class Point { private int x, y; public Point(int x, int y) {...} public int getX() {...} public void setX(int x) {...} public int getY() {...} public void setY(int y) {...} } Naming © GDP Labs 2017

Slide 23

Slide 23 text

23 public class Circle { double radius; String color; public double getRadius() {...} public String fetchColor() {...} public double retrieveArea() {...} } GET OR FETCH OR RETRIEVE? Naming © GDP Labs 2017

Slide 24

Slide 24 text

24 Pick one word per concept public class Circle { double radius; String color; public double getRadius() {...} public String getColor() {...} public double getArea() {...} } Naming © GDP Labs 2017

Slide 25

Slide 25 text

25 for (int i=0; i<34; i++) { s += (t[i]*4)/5; } if single-letter names and numeric constants are used across body of text, they are not easy to locate NOT EASY TO SEARCH Naming © GDP Labs 2017

Slide 26

Slide 26 text

26 Use searchable names int realDaysPerIdealDay = 4; const int WORK_DAYS_PER_WEEK = 5; int sum = 0; for (int i=0; i < NUMBER_OF_TASKS; i++) { int realTaskDays = taskEstimate[i] * realDaysPerIdealDay; int realTaskWeeks = (realdays / WORK_DAYS_PER_WEEK); sum += realTaskWeeks; } Naming © GDP Labs 2017

Slide 27

Slide 27 text

27 Function The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that. - Robert C. Martin ” “ © GDP Labs 2017

Slide 28

Slide 28 text

28 public static String renderPageWithSetupsAndTeardowns( PageData pageData, boolean isSuite ) throws Exception { boolean isTestPage = pageData.hasAttribute("Test"); if (isTestPage) { WikiPage testPage = pageData.getWikiPage(); StringBuffer newPageContent = new StringBuffer(); includeSetupPages(testPage, newPageContent, isSuite); newPageContent.append(pageData.getContent()); includeTeardownPages(testPage, newPageContent, isSuite); pageData.setContent(newPageContent.toString()); } return pageData.getHtml(); } TOO BIG

Slide 29

Slide 29 text

29 Do one thing public static String renderPageWithSetupsAndTeardowns( PageData pageData, boolean isSuite) throws Exception { if (isTestPage(pageData)) includeSetupAndTeardownPages(pageData, isSuite); return pageData.getHtml(); } Function is doing more than “one thing” if you can extract another function from it with a name that is not merely a restatement Function © GDP Labs 2017

Slide 30

Slide 30 text

30 public boolean set(String attribute, String value); if (set(”username”, ”unclebob”)) Is it asking whether the “username” attribute was previously set to “unclebob”? Or is it asking whether the “username” attribute was successfully set to “unclebob”? Function © GDP Labs 2017

Slide 31

Slide 31 text

31 Do something or answer something, but not both if (attributeExists(”username”)) { setAttribute(”username”, ”unclebob”); } Function © GDP Labs 2017

Slide 32

Slide 32 text

32 Transformational function should appear as the return value StringBuffer transform(StringBuffer in) void transform(StringBuffer out) These implementation simply returns the input argument Function © GDP Labs 2017

Slide 33

Slide 33 text

33 Avoid too many function parameters Car createCar(Wheel wheel, Engine engine); Car createCar(float wheelDiameter, float wheelColor, float wheelMaterial, float wheelManufacturer, String engineType, String engineColor, String engineManufacturer); Too many parameters will make function hard to be understood and maintained. Function © GDP Labs 2017

Slide 34

Slide 34 text

34 Comments Good code is its own best documentation. As you’re about to add a comment, ask yourself, ‘How can I improve the code so that this comment isn’t needed?' - Steve McConnell ” “ © GDP Labs 2017

Slide 35

Slide 35 text

// Throws an exception // if the timeout is reached. public synchronized void waitForClose( final long timeoutMillis) throws Exception { if(!closed) { wait(timeoutMillis); if(!closed) throw new Exception( "MockResponseSender could not be closed"); } } 35 Comments © GDP Labs 2017 Redundant Comments The comment is not more informative than the code

Slide 36

Slide 36 text

/** * Returns the day of the month. * @return the day of the month. */ public int getDayOfMonth() { return dayOfMonth; } 36 Comments © GDP Labs 2017 Noise Comments Only restate the obvious and provide no new information

Slide 37

Slide 37 text

Explain yourself in code 37 // Check to see if the employee is eligible for full benefits if ((employee.flags & HOURLY_FLAG) && (employee.age > 65)) if (employee.isEligibleForFullBenefits()) Comments © GDP Labs 2017

Slide 38

Slide 38 text

// format matched kk:mm:ss EEE, MMM dd, yyyy Pattern timeMatcher = Pattern.compile("\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*"); INFORMATIVE public void writeJournal(Diary diary) { for (Thread thread : threadList) { thread.run(() -> { // Prevent original object to be modified Diary copyOfDiary = new Diary(diary); write(copyOfDiary); }); } } EXPLANATION OF INTENT 38 Comments © GDP Labs 2017

Slide 39

Slide 39 text

public static SimpleDateFormat makeStandardHttpDateFormat() { //SimpleDateFormat is not thread safe, //so we need to create each instance independently. SimpleDateFormat df = new SimpleDateFormat( ”EEE, dd MMM yyyy HH:mm:ss z”); df.setTimeZone(TimeZone.getTimeZone(”GMT”)); return df; } WARNING OF CONSEQUENCES 39 Comments © GDP Labs 2017

Slide 40

Slide 40 text

Formatting 40 © GDP Labs 2017

Slide 41

Slide 41 text

Variables Should be declared as close to their usage as possible 41 public void paySalary(Employee employee) { float bonus; float totalSalary; bonus = calculateBonus(employee.getSalary()); totalSalary = bonus + employee.getSalary(); sendMoney(employee, totalSalary); } Formatting © GDP Labs 2017

Slide 42

Slide 42 text

42 Variables Should be declared as close to their usage as possible Formatting public void paySalary(Employee employee) { float bonus; bonus = calculateBonus(employee.getSalary()); float totalSalary; totalSalary = bonus + employee.getSalary(); sendMoney(employee, totalSalary); } © GDP Labs 2017

Slide 43

Slide 43 text

43 public class Employee { private String name; private String id; private float salary; public void getName() { … } } Instance Variables Should be declared at the top of the class Formatting © GDP Labs 2017

Slide 44

Slide 44 text

44 public void paySalary() { calculateBonus(salary); } private float calculateBonus(float salary) { return (salary / 10); } Dependent Functions Should be vertically close, and the caller should be above the called Formatting © GDP Labs 2017

Slide 45

Slide 45 text

45 public class Employee { public void payTax() { } public void payOverdueTax(Date date) { } public void increaseSalary() { } public void decreaseSalary() { } } Conceptual Affinity Certain bits of code want to be near other bits Formatting © GDP Labs 2017

Slide 46

Slide 46 text

public float volume (float length,float width,float height) { // code } 46 public float volume(float length, float width, float height) { // code } Space Formatting © GDP Labs 2017

Slide 47

Slide 47 text

47 public class WebService { private Request request; private Response response; private FitnesseContext context; protected long requestTimeLimit; } Horizontal Alignment Formatting © GDP Labs 2017 DIFFICULT TO MAINTAIN

Slide 48

Slide 48 text

public class WebService { private Request request; private Response response; private FitnesseContext context; protected long requestTimeLimit; } 48 Horizontal Alignment Formatting © GDP Labs 2017

Slide 49

Slide 49 text

public String functionName() {return "";} public String functionName() { return ""; } 49 Indentation Formatting © GDP Labs 2017

Slide 50

Slide 50 text

Objects and Data Structures 50 © GDP Labs 2017

Slide 51

Slide 51 text

51 public class Point { public double x; public double y; } Data structures expose data and have no behavior Data structures make it easy to add functions without the need to modify existing structures. Objects and Data Structures © GDP Labs 2017

Slide 52

Slide 52 text

52 public class Vehicle { public getFuelCapacity() {...} public getPercentageFuelRemaining() {...} } Object expose behavior and hide data Objects make it easy to add classes without the need to modify existing functions. Objects and Data Structures © GDP Labs 2017

Slide 53

Slide 53 text

53 Law of Demeter Given method f of class C, f should only call methods of: ● C ● An Object created by f ● An Object passed as an argument to f ● An instance variable of C Objects and Data Structures © GDP Labs 2017

Slide 54

Slide 54 text

54 public class Vehicle { public getFuelCapacity() {...} public getPercentageFuelRemaining() { return (fuel / getFuelCapacity() * 100); } } Law of Demeter Given method f of class C, f should only call methods of: ● C Objects and Data Structures © GDP Labs 2017

Slide 55

Slide 55 text

55 public void registryEmployee() { Employee newEmployee = new Employee(); registry.createEmplyeeId(newEmployee); employeeIdList.add(newEmployee.getId()); } Law of Demeter Given method f of class C, f should only call methods of: ● An Object created by f Objects and Data Structures © GDP Labs 2017

Slide 56

Slide 56 text

56 public void calculateEmployeeBonus(Employee employee) { return (employee.getSalary() / 10); } Law of Demeter Given method f of class C, f should only call methods of: ● An Object passed as an argument to f Objects and Data Structures © GDP Labs 2017

Slide 57

Slide 57 text

57 public class Car { private Engine engine; public String getCarFuelType() { return engine.getFuelType(); } } Law of Demeter Given method f of class C, f should only call methods of: ● An instance variable of C Objects and Data Structures © GDP Labs 2017

Slide 58

Slide 58 text

Slide 59

Slide 59 text

if (deletePage(page) == E_OK) { if (registry.delete(page.reference) == E_OK) { logger.log(“page deleted”); } else { logger.log(“delete registry failed”); } } else { logger.log(“delete failed”); } 59 Prefer exceptions to return error code Error Handling © GDP Labs 2017

Slide 60

Slide 60 text

try { deletePage(page); registry.delete(page.reference); } catch (Exception e) { logger.log(e.getMessage()); } 60 Prefer exceptions to return error code Error Handling © GDP Labs 2017

Slide 61

Slide 61 text

● Functions should do one thing and error handling is one thing ○ Implement the normal flow of the function ● Don’t return NULL ○ So other function doesn’t need to implement error handling ● Don’t pass NULL ○ So other function doesn’t need to implement error handling 61 Error Handling © GDP Labs 2017

Slide 62

Slide 62 text

Slide 63

Slide 63 text

63 public class TimeCalculator { public static final int TIME = 25; // public static constant public static int DURATION = 25; // private static variables private int now; // private instance variables public int addTime(int time) { // public functions … configTime = getConfigTime() … } private int getConfigTime() { … } // private utilities } Class Class Organization Declare the constants, variables, and methods in this order: © GDP Labs 2017

Slide 64

Slide 64 text

Classes should be small! ● The first rule is that they should be small ● The second rule is that they should be smaller than that Single Responsibility Principle (SRP) ● A class or module should have one, and only one, reason to change ● SRP is one of the more important concept in OO design 64 Class © GDP Labs 2017

Slide 65

Slide 65 text

Refactored: 65 public class TimeCalculator { … public int addTime(int time) { … } … } public class UserConfiguration { … public int getConfigTime() { … } … } Class © GDP Labs 2017

Slide 66

Slide 66 text

Cohesion ● Classes should have a small number of instance variables ● The more variables (or class modules) a method manipulates the more cohesive that method is to its class. ● A class in which each variable is used by each method is maximally cohesive. ● Maintaining cohesion results in many small classes 66 Class © GDP Labs 2017

Slide 67

Slide 67 text

67 public class CustomStack { private int topOfStack = 0; private int duration = 100; List elements = new LinkedList(); public int size() { … } public void push(int element) { … } public int pop() throws PoppedWhenEmpty { … } public void sleep() { TimeUnit.SECONDS.sleep(duration); } public void log() { … logger.log(Level.WARNING, ”This is a warning!”); … } … } LOW COHESION © GDP Labs 2017

Slide 68

Slide 68 text

68 public class Stack { private int topOfStack = 0; List elements = new LinkedList(); public int size() { return topOfStack; } public void push(int element) { topOfStack++; elements.add(element); } public int pop() throws PoppedWhenEmpty { if (topOfStack == 0) throw new PoppedWhenEmpty(); int element = elements.get(--topOfStack); elements.remove(topOfStack); return element; } } HIGH COHESION © GDP Labs 2017

Slide 69

Slide 69 text

Slide 70

Slide 70 text

● Runs all the tests. To make it easy, make sure: ○ Low coupling ○ SRP ● Contains no duplication (Refactoring) ● Expresses the intent of the programmer (Refactoring) ○ Easy to read and understand ● Minimizes the number of classes and methods (Refactoring) ○ But don’t take it too far 70 Emergence © GDP Labs 2017

Slide 71

Slide 71 text

71 public void scaleToOneDimension(…) { … RenderedOp newImage = ImageUtilities.getScaledImage( image, scalingFactor, scalingFactor); image.dispose(); System.gc(); image = newImage; } public synchronized void rotate(int degrees) { RenderedOp newImage = ImageUtilities.getRotatedImage( image, degrees); image.dispose(); System.gc(); image = newImage; } DUPLICATION © GDP Labs 2017

Slide 72

Slide 72 text

72 public void scaleToOneDimension(…) { … replaceImage(ImageUtilities.getScaledImage( image, scalingFactor, scalingFactor)); } public synchronized void rotate(int degrees) { replaceImage(ImageUtilities.getRotatedImage( image, degrees)); } private void replaceImage(RenderedOp newImage) { image.dispose(); System.gc(); image = newImage; } DUPLICATION REFACTORED © GDP Labs 2017

Slide 73

Slide 73 text

Give a man a fish and you feed him for a day; teach a man to fish and you feed him for a lifetime. - Anne Thackeray Ritchie 73

Slide 74

Slide 74 text

74 Conclusion 1. Practice Is there a set of simple practices that can replace experience? Clearly not 2. Beware of Bad Code or Code Smell 3. Refactor © GDP Labs 2017

Slide 75

Slide 75 text

Slide 76

Slide 76 text

A programmer who writes clean code is an artist who can take a blank screen through a series of transformations until it is an elegantly coded system 76

Slide 77

Slide 77 text

77 Thank You Q & A speakerdeck.com/gdplabs

Slide 78

Slide 78 text

78 We Are Hiring! [email protected]

Slide 79

Slide 79 text

Slide 80

Slide 80 text

Slide 81

Slide 81 text

● Large class ● Feature envy ● Inappropriate intimacy ● Refused bequest ● Lazy class ● Excessive use of literals ● Cyclomatic complexity ● Data clump ● Orphan variable or constant class 81 ● Duplicated code ● Too many parameters ● Long method ● Excessive return of data ● Excessively long identifiers ● Excessively short identifiers Code Smells © GDP Labs 2017

Slide 82

Slide 82 text

Single-responsibility principle Open-closed principle Liskov substitution principle Interface segregation principle Dependency Inversion Principle 82 OO Design Principles: S.O.L.I.D S O L I D © GDP Labs 2017