Clean Code

Lunch and Learn

June 10, 2016

Reid Harrison

Georgia Tech, August '13

Senior Software Engineer

Digital Engineering - Platforms & Frameworks - Chassis

A craftsman or artisan is a skilled manual worker who makes items that may be functional or strictly decorative.

How is software a craft?

How is software different from typical crafts?

What makes programmers artisans?

Why is it important to improve in your craft?

What are important qualities of a skilled craftsman?

How can you improve your crafting skills?

What is "Software Craftsmanship?"

try {
    Assert(Life.Real);
    Assert(Life.Fantasy);
} catch(LandSlideException ex) {
    #region Reality
    while(true) {
        character.Eyes.ForEach(eye => eye.Open().Orient(Direction.Sky).See());
        self.Wealth = null;
        self.Sex = Sex.Male;

        if(self.ComeDifficulty == Difficulty.Easy && 
                self.GoDifficulty == Difficulty.Easy && 
                self.High < 0.1 && self.Low < 0.1) {
            self.Sympathies.Clear();
            switch(wind.Direction) {
                case Direction.North:
                case Direction.East:
                case Direction.South:
                case Direction.West:
                default:
                    piano.Play();
                    break;
            }
        }
    }
 #endregion
}

The Art of Software

Writing code is similar to painting a picture. A programmer who writes clean code is an artist who can guide a blank screen through a series of transformations until it is an elegantly coded system.

  • ​Easy to recognize a good painting
  • Hard to paint a good picture

 

Programmers are authors – responsible for communicating well with their readers.

  • The ratio spent reading vs. writing code is usually at least 10:1
  • Programmers write for readers who will critique their effort and skill

The Science of Software

What do these have in common?

  • UI/UX Design
  • Animating a Movie
  • Engineering a Bridge
  • Studying Quantum Entanglement
  • Programming

Break the bad habit of:

"I don't know what to do so I'll just write the code and see what happens."

Scientific Method

Science

UI/UX Design

Programming

Bridge Building

Ask a Question

Background Research

Construct Hypothesis

Test with Experiment

Analyze Data &

Draw Conclusions

Communicate Results

Identify a User Problem

User Research and Interviews

Construct

Prototype

User Testing and Feedback

Incorporate Feedback

Hand Off / Implement Design

Proposal Request

Survey Site

Design to Specifications

CAD and Physical Model

Improve Structure

Build Bridge

Technical Challenge

Gather Requirements

Plan Stories and Design System

PoC, User and Perf Testing

Improve Structure

QS Validation and Release

From Journeyman to Master

As a student:

  • Solve it
  • Test it
  • Fix it
  • Turn it in
  • Forget about it

As a professional:

  • Solve it
    • Make it easy to understand
    • Make it easy to use
    • Make it easy to change
  • Test it
  • Fix it
  • Review it
  • Commit it
  • Test it
  • Release it
  • Support it

Mantras of a Software Craftsman

  • Broken windows
    • ​One broken window, left un-repaired, instills in the community a sense of abandonment.
  • Boy Scout rule
    • ​"Leave the campground cleaner than you found it."
  • Tests are your parachute
    • Keep them free of holes and obstructions.

Your Knowledge Portfolio

Your knowledge and experience are your most important professional assets.

Unfortunately, they're expiring assets.

  • Invest regularly
  • Diversify
  • Manage risk
  • Buy low, sell high
  • Review and rebalance

Expand your knowledge portfolio regularly; not only with technical expertise but also industry experience.

Care about your craft

Think about your work

Continuously make small improvements

throw new EPFSystemException("Structure exceeds two levels " + objEntry + " " + json);
if (l2Temp != null && l2Temp.isValueNode()) {
  logger.debug("Is nested value node {} stack id {}", objEntry, stackId);
  buildAndAddEvtDetail(event, objEntry, l2Temp.asText(), stackId);
}
else {
  throw new EPFSystemException("Structure exceeds two levels " + objEntry + " " + json);
}
for (int x = 0; x < anode.size(); x++) {
  Iterator<String> level2 = anode.get(x).fieldNames();
  while (level2.hasNext()) {
    String objEntry = level2.next();
    logger.debug("Processing nested object {} stack id {}", objEntry, stackId);
    JsonNode l2Temp = anode.get(objEntry);
    if (l2Temp != null && l2Temp.isValueNode()) {
      logger.debug("Is nested value node {} stack id {}", objEntry, stackId);
      buildAndAddEvtDetail(event, objEntry, l2Temp.asText(), stackId);
    }
    else {
      throw new EPFSystemException("Structure exceeds two levels " + objEntry + " " + json);
    }
  }
  stackId++;
}
if (temp.isValueNode()) {
  logger.debug("Found value node, building detail {}", fname);
  buildAndAddEvtDetail(event, fname, temp.asText(), 0);
}
else if (temp.isArray()) {
  logger.debug("Found array, building detail {}", fname);
  ArrayNode anode = (ArrayNode) temp;
  if (anode.size() > 0) {
    JsonNode obj = findNonNullObject(anode);
    if (obj == null || obj.isValueNode()) {
      logger.debug("Found Array of value nodes {} size {}", fname, anode.size());
      int stackId = 1;
      for (int x = 0; x < anode.size(); x++) {
        if (anode.get(x) != null) {
          buildAndAddEvtDetail(event, fname, anode.get(x).asText(), stackId);
        }
        stackId++;
      }
    }
    else if (obj.isObject()) {
      logger.debug("Found Array of objects {} size {}", fname, anode.size());
      int stackId = 1;
      for (int x = 0; x < anode.size(); x++) {
        Iterator<String> level2 = anode.get(x).fieldNames();
        while (level2.hasNext()) {
          String objEntry = level2.next();
          logger.debug("Processing nested object {} stack id {}", objEntry, stackId);
          JsonNode l2Temp = anode.get(objEntry);
          if (l2Temp != null && l2Temp.isValueNode()) {
            logger.debug("Is nested value node {} stack id {}", objEntry, stackId);
            buildAndAddEvtDetail(event, objEntry, l2Temp.asText(), stackId);
          }
          else {
            throw new EPFSystemException("Structure exceeds two levels " + objEntry + " " + json);
          }
        }
        stackId++;
      }
    }
    else {
      throw new EPFSystemException("error - found unexpected object type found " + fname + " " + json);
    }
  }
}
else if (temp.isObject()) {
  logger.debug("Found object, building detail {}", fname);
  Iterator<String> level1 = temp.fieldNames();
  while (level1.hasNext()) {
    String objEntry = level1.next();
    logger.debug("Processing object {}", objEntry);
    JsonNode l2Temp = temp.get(objEntry);
    if (l2Temp != null && l2Temp.isValueNode()) {
      logger.debug("Is a value node {}", objEntry);
      buildAndAddEvtDetail(event, objEntry, l2Temp.asText(), 0);
    }
    else {
      throw new EPFSystemException("error - found unexpected object type found " + objEntry + " " + json);
    }
  }
}

The Perils of Messy Code

Mess builds, productivity of team decreases

Under pressure, the team makes more messes

Entropy - The amount of disorder in a system. When disorder increases in software, it is called "code rot".

The only way to develop quickly and meet deadlines is to keep the code as clean as possible at all times.

http://xkcd.com/844/

def test():

    #run speedtest-cli
    print 'running test'
    a = os.popen("python /home/pi/speedtest/speedtest-cli --simple").read()
    print 'ran'
    #split the 3 line result (ping,down,up)
    lines = a.split('\n')
    print a
    ts = time.time()
    date =datetime.datetime.fromtimestamp(ts).strftime('%Y-%m-%d %H:%M:%S')
    #if speedtest could not connect set the speeds to 0
    if "Cannot" in a:
            p = 100
            d = 0
            u = 0
    #extract the values for ping down and up values
    else:
            p = lines[0][6:11]
            d = lines[1][10:14]
            u = lines[2][8:12]
    print date,p, d, u
    #save the data to file for local network plotting
    out_file = open('/var/www/assets/data.csv', 'a')
    writer = csv.writer(out_file)
    writer.writerow((ts*1000,p,d,u))
    out_file.close()

    #connect to twitter
    TOKEN=""
    TOKEN_KEY=""
    CON_SEC=""
    CON_SEC_KEY=""

    my_auth = twitter.OAuth(TOKEN,TOKEN_KEY,CON_SEC,CON_SEC_KEY)
    twit = twitter.Twitter(auth=my_auth)

    #try to tweet if speedtest couldnt even connet. Probably wont work if the internet is down
    if "Cannot" in a:
            try:
                    tweet="Hey @Comcast @ComcastCares why is my internet down? I pay for 150down\\10up in Washington DC? #comcastoutage #comcast"
                    twit.statuses.update(status=tweet)
            except:
                    pass

    # tweet if down speed is less than whatever I set
    elif eval(d)<50:
            print "trying to tweet"
            try:
                    # i know there must be a better way than to do (str(int(eval())))
                    tweet="Hey @Comcast why is my internet speed " + str(int(eval(d))) + "down\\" + str(int(eval(u))) + "up when I pay for 150down\\10up in Washington DC? @ComcastCares @xfinity #comcast #speedtest"
                    twit.statuses.update(status=tweet)
            except Exception,e:
                    print str(e)
                    pass
    return
        
if __name__ == '__main__':
    test()
    print 'completed'

What is messy code?

  • Smells bad
  • Follows anti-patterns
  • Includes spaghetti code
  • Contains duplication
  • Comprises complexity
  • Obscures intent
  • Complicates logical flow
  • Perpetuates inconsistencies
  • Becomes unmaintained

Messy code is a symptom of an underlying poor design or other problem.

What is clean code?

  • Reads like well-written prose
  • Clarifies the designer's intent
  • Sustains simple and direct modules
  • Preserves straightforward logical flow
  • Employs efficient abstractions
  • Fosters maintenance and enhancement
  • Provides one way to do one thing
  • Presents a clear and minimal API
  • Includes unit and acceptance tests
  • Requires minimal dependencies

Clean code is the reward for elegant design, planning, and execution.

if (appObj.isDsclsrOnlineRequiredInd() || appObj.isDsclsrCreditScoreRequiredInd()) {
    if (appObj.isDsclsrOnlineRequiredInd()) {
        // Pull account disclosure
        setDisclosureVariableFromContent(AOD, aodVariableReplacer,
            appObject.getDecisionedOfferID(), model, evalContext);
    }
            
    if (appObject.isDsclsrCreditScoreRequiredInd() && appObject.getMinCreditScore() != null 
            && appObject.getMinCreditScore().getBureauCode()!=null) {
        // Pull credit disclosure
        if(appObject.getMinCreditScore().getBureauCode().equals("")){
            setDisclosureVariableFromContent(CSD, csdVariableReplacer,
               NO_HIT_KEY , model, evalContext);
        }else{
            setDisclosureVariableFromContent(CSD, csdVariableReplacer,
                appObject.getMinCreditScore().getBureauCode() , model, evalContext);
        }
    }
}

if (appObject.getApplicant() != null && appObject.getApplicant().get(0) != null ) { 
    userInfo.setFirstName(appObject.getApplicant().get(0).getFirstName());
    userInfo.setLastName(appObject.getApplicant().get(0).getLastName());			
    userInfo.setLangCode(appObject.getApplicant().get(0).getLangCode());
    userInfo.setSsoId(null);
    userInfo.setUserId(appObject.getApplicant().get(0).getCustomerID());
			
    if (appObject.getApplicant().get(0).getEmailAddresses() != null  &&
            appObject.getApplicant().get(0).getEmailAddresses().size() > 0) {	
        userInfo.setEmailAddress(appObject.getApplicant().get(0).getEmailAddresses().get(0));
    }	
}

Coupling and Cohesion

With tightly coupled systems:

  1. A change in one module forces changes in others
  2. Modules are harder to reuse
  3. Modules are harder to test

With low cohesion systems:

  1. Modules are complex with more operations
  2. Modules are less maintainable and harder to work with

Coupling is a measure of how closely connected two routines or modules are.

Cohesion is a measure of how strongly related each piece of functionality is.

Coupling and Cohesion have an inverse relationship

Orthogonality

  • When components are highly interdependent, there is no such thing as a quick, local fix.

  • Orthogonal software provides increased productivity and decreased risk because developers never have to worry about side-effects of making changes.

  • Orthogonal components are easier to swap meaning less dependence on a specific library or vendor.

Two or more modules are orthogonal if

changes in one do not affect the other.

SOLID Design Principles

  • Single Responsibility Principle (SRP)

  • Open/Closed Principle (OCP)

  • Liskov Substitution Principle (LSP)

  • Interface Segregation Principle (ISP)

  • Dependency Inversion Principle (DIP)

public Map<String, AppSetting> getAllSettings(DomainInfo domainInfo) {
    Map<String, AppSetting> settingMap = new Hashtable<String, AppSetting>();
    if (domainInfo.getStorageType().equalsIgnoreCase(AppSettingConstants.DB_SYSTEM_STORAGE)) {
        try {
            readItFromDB(domainInfo, settingMap);
        }
        catch (RuntimeException dbException) {
            readItFromFileSystem(domainInfo, settingMap);
        }
    }
    else if (domainInfo.getStorageType().equalsIgnoreCase(AppSettingConstants.FILE_SYSTEM_STORAGE)) {
        readItFromFileSystem(domainInfo, settingMap);
    }
    else {
        return null;
    }

    return settingMap;
}
private void readItFromDB(DomainInfo domainInfo, Map<String, AppSetting> settingMap) {
    Map<String, Settings> settingFromDB;

    if (isMongoDB()) {
        settingFromDB = mongoDBSettingsStore.readAll(domainInfo.getAppName(), domainInfo.getDomainIndentifier());
    }
    else {
        settingFromDB = jdbcSettingsStore.readAll(domainInfo.getAppName(), domainInfo.getDomainIndentifier());
    }

    loadSettingsMap(settingFromDB, settingMap);

    // write it to the file system for backup activities:
    try {
        writeSettingsOnFilesystemForBackup(settingFromDB);
    }
    catch (Exception excpetion) {
        logger.error("Could not write to file system path: " + domainInfo.getSettingsFilePath(), excpetion);
    }
}
public void toggleSettings(DomainInfo DomainInfo, String regionIdentifier, String toggle, String settingName, String settingGroup) {
    if (DomainInfo.getStorageType().equalsIgnoreCase(AppSettingConstants.DB_SYSTEM_STORAGE)) {
        if (toggle.equalsIgnoreCase("enable")) {
            if (isMongoDB()) {
                mongoDBSettingsStore.enable(settingName, settingGroup, regionIdentifier);
            }
            else {
                jdbcSettingsStore.enable(settingName, settingGroup, regionIdentifier);
            }
        }
        else if (toggle.equalsIgnoreCase("disable")) {
            if (isMongoDB()) {
                mongoDBSettingsStore.disable(settingName, settingGroup, regionIdentifier);
            }
            else {
                jdbcSettingsStore.disable(settingName, settingGroup, regionIdentifier);
            }
        }
        else {
            Message error = new Message(appConfiguration.getString(AppSettingConstants.INVALID_FLIPPING_OPTION_ERROR_CODE));
            error.setInputParams(appConfiguration.getString(AppSettingConstants.INVALID_FLIPPING_OPTION_MESSAGE));
            throw new RuntimeException(AppSettingConstants.INVALID_FLIPPING_OPTION_MESSAGE);
        }
    }
    else if (DomainInfo.getStorageType().equalsIgnoreCase(AppSettingConstants.FILE_SYSTEM_STORAGE)) {
        if (toggle.equalsIgnoreCase("enable")) {
            fileSystemStore.enable(settingName);
        }
        else if (toggle.equalsIgnoreCase("disable")) {
            fileSystemStore.disable(settingName);
        }
        else {
            Message error = new Message(appConfiguration.getString(AppSettingConstants.INVALID_FLIPPING_OPTION_ERROR_CODE));
            error.setInputParams(appConfiguration.getString(AppSettingConstants.INVALID_FLIPPING_OPTION_MESSAGE));
            throw new RuntimeException(AppSettingConstants.INVALID_FLIPPING_OPTION_MESSAGE);
        }
    }
    else {
        throw new ApplicationException(AppSettingConstants.INVALID_SYSTEM_OF_RECORD_OPT);
    }
}

Single Responsibility Principle

 

  • The single responsibility should be entirely encapsulated by its context

  • A unit of code should have only one reason to change

 

  • Keeps a class focused (high cohesion)

  • Lowers coupling

A class or module should be defined by one, and only one, responsibility.

Interface Segregation Principle

Interface-based design:

  • The foundation of APIs
  • Design from the outside in
  • Write the interfaces first
  • Practice TDD

 

Keep interfaces focused and segregated by purpose. Many client-specific interfaces are better than one general purpose interface.

   public String generateMessage(StreamMessageContext ctx, Object... data) {
        int size = ((data == null) ? 1 : data.length + 1);
        
        // serialize each object
        int i = 1;
        String[] jsonEntries = new String[size];
        jsonEntries[0] = DataConversionSupport.serializeToJsonString(ctx);
        for (Object datum : data) {
            jsonEntries[i] = DataConversionSupport.serializeToJsonString(datum);
            ++i;
        }
        
        // then aggregate all the serializations together
        StringBuilder sb = new StringBuilder();
        sb.append('{').append('\"').append(ctx.getClass().getSimpleName()).append("\":").append(jsonEntries[0]); // the key
        for (int j = 0; j < i - 1; ++j) {
            sb.append(',\"').append(data[j].getClass().getSimpleName()).append("\":"); // item name
            sb.append(jsonEntries[j + 1]); // item json
        }
        sb.append('}');
        return sb.toString();
    }
    
    public String generateMessage(StreamMessageContext ctx, String[] dataKeys, Object... data) {
        int size = ((data == null) ? 1 : data.length + 1);
        
        // serialize each object
        int i = 1;
        String[] jsonEntries = new String[size];
        jsonEntries[0] = DataConversionSupport.serializeToJsonString(ctx);
        for (Object datum : data) {
            jsonEntries[i] = DataConversionSupport.serializeToJsonString(datum);
            ++i;
        }
        
        // build up keys
        String[] keys = processDataKeys(dataKeys, data);
        
        // then aggregate all the serializations together
        StringBuilder sb = new StringBuilder();
        sb.append('{').append('\"').append(ctx.getClass().getSimpleName()).append("\":").append(jsonEntries[0]); // the key
        for (int j = 0; j < i - 1; ++j) {
            sb.append(',\"').append(keys[j]).append("\":"); // item name
            sb.append(jsonEntries[j + 1]); // item json
        }
        sb.append('}');
        return sb.toString();
    }

The Evils of Duplication

  • Requirements, understanding, and environments change. Your code will need to change with it.

  • It's not a matter of if you'll remember, but when you'll forget.

DRY - Don't Repeat Yourself

  • Imposed duplication
  • Inadvertent duplication
  • Impatient duplication
  • Inter-developer duplication

Follow the Rules

Actually, follow the Principles

Rules are attempts to standardize agreement of how we follow Principles.

Rules change over time, teams, and environments.

The underlying Principles stay the same.

Breaking rules can be fine but breaking principles will cost down the line.

Rule: Don't tell lies

Principle: Honesty and Kindness

Naming Conventions

Intention revealing

int d; // elapsed time in days
int daysSinceCreation;

Avoid disinformation

Account[] accountList;
boolean notActive;

Make meaningful distinctions

void arrayCopy(char[] a1, char[] a2);
void arrayCopy(char[] source, char[] destination);

Pronounceable

String evtStCd, evtAudtg;

Searchable

Date date, transactionDate;
public class RequestBuilder { ...

Solution/Problem relevant

int tableUsage, loadFactor;
Node tortoise, hare;

Function Conventions

  • Small
  • Do only one thing
  • One level of abstraction per function
  • Descriptive names

 

int getStatus();
int getResponseStatusCode();
  • Minimal parameters
  • Don't pass codes
  • Don't return null
  • Verbs not nouns
int cardCount();
int getCardCount();

void newCard();
void addNewCard();

Code Formatting Conventions

https://google-styleguide.googlecode.com/svn/trunk/javaguide.html

http://lars-lab.jpl.nasa.gov/JPL_Coding_Standard_Java.pdf

Common rules:

  • Documentation (JavaDoc) conventions
  • Upper-case class names; camel-case variable names
  • New line after conditionals; always using braces
  • One variable per declaration
  • No chained method calls
  • Store common references for reuse
  • Encapsulate and use positive conditionals
  • Declare abstract type, instantantiate implementation

Follow team and company rules

Commenting Conventions

"Programs must be written for people to read and only incidentally for machines to execute" - Hal Abelson

  • The only "comment" guaranteed to be accurate is the code its self
  • Comments have to be maintained and updated same as the code
  • The more complex the comments are, the more likely they are to be wrong or out of date

The best solution is to write easily comprehensible code.

On the Grind

  • Be aware
  • Don't be "lazy"
  • Seek feedback
    • Code reviews
    • Books and Blogs
    • Retrospectives
  • Continuously improve
  • PRACTICE!
Made with Slides.com