KEMBAR78
Clean coding-practices | PDF
Clean Coding Practices for Java Developers




 Clean Coding Practices for Java Developers




                                      John	
  Ferguson	
  Smart
                               john.smart@wakaleo.com
                               	
  h2p://www.wakaleo.com
                                          Twi2er:	
  wakaleo
So who is this guy, anyway?
               Consulta
                       nt
               Trainer
              Mentor
              Author
             Speaker
             Coder
   John Fer
            guson S
                    mar t
Who	
  are	
  you	
  wri;ng	
  code	
  for,	
  anyway?
Who	
  are	
  you	
  wri;ng	
  code	
  for,	
  anyway?




                                The	
  computer?
Who	
  are	
  you	
  wri;ng	
  code	
  for,	
  anyway?




                             Other	
  developers?
Who	
  are	
  you	
  wri;ng	
  code	
  for,	
  anyway?




                                Your	
  next	
  boss?
Clean Coding Practices for Java Developers



        Why	
  is	
  clean	
  code	
  so	
  important?
Easier	
  to	
  Understand




Easier	
  to	
  Change




            Cheaper	
  to	
  Maintain
Choose	
  your	
  names	
  well




"What's in a name? That which we call a rose
  By any other name would smell as sweet."
                 Romeo and Juliet (II, ii, 1-2)
   Really?
Use	
  a	
  common	
  vocabulary

getCustomerInfo()
getClientDetails()
getCustomerRecord()
...
    Which one do I use? Are they the same?



           getCustomer()
                Choose one and stick to it
Use	
  meaningful	
  names

int days;




              What does this represent?


int daysSinceCreation;

 int daysSinceLastModification;

    int durationInDays;

                         These are all more meaningful choices
Don’t	
  talk	
  in	
  code

class DtaRcrd102 {
    private Date genymdhms;
    private Date modymdhms;
    private final String pszqint = “102”;
    ...
}                                     Huh?

class Customer {
  private Date generationTimestamp;
  private Date modificationTimestamp;
  private final String recordId = “102”
  ...
}
But	
  don’t	
  state	
  the	
  obvious

List<Client> clientList;


                        Is ‘List’ significant or just noise?




List<Client> clients;

  List<Client> regularClients;

     List<Client> newClients;
In	
  short:	
  Don’t	
  make	
  me	
  think!
                                                        What does this do?




                                                   Clearer parameter name
More explicit method name


                                                       What are we counting?




 Method call rather than
  boolean expression
Make	
  your	
  code	
  tell	
  a	
  story
High-Level
Methods	
  should	
  be	
  small                                      Refactoring
  public void generateAggregateReportFor(final List<StoryTestResults> storyResults,
                                         final List<FeatureResults> featureResults) throws IOException {
      LOGGER.info("Generating summary report for user stories to "+ getOutputDirectory());

      copyResourcesToOutputDirectory();

      Map<String, Object> storyContext = new HashMap<String, Object>();
      storyContext.put("stories", storyResults);
      storyContext.put("storyContext", "All stories");
      addFormattersToContext(storyContext);
      writeReportToOutputDirectory("stories.html",
                                   mergeTemplate(STORIES_TEMPLATE_PATH).usingContext(storyContext));

      Map<String, Object> featureContext = new HashMap<String, Object>();
      addFormattersToContext(featureContext);
      featureContext.put("features", featureResults);
      writeReportToOutputDirectory("features.html",
                                   mergeTemplate(FEATURES_TEMPLATE_PATH).usingContext(featureContext));

      for(FeatureResults feature : featureResults) {
          generateStoryReportForFeature(feature);
      }

      generateReportHomePage(storyResults, featureResults);


                                          Code hard to understand
      getTestHistory().updateData(featureResults);

      generateHistoryReport();
  }
High-Level
Methods	
  should	
  be	
  small                                       Refactoring
  public void generateAggregateReportFor(final List<StoryTestResults> storyResults,
                                         final List<FeatureResults> featureResults) throws IOException {
      LOGGER.info("Generating summary report for user stories to "+ getOutputDirectory());

       copyResourcesToOutputDirectory();

      private void Object> storyContext = new HashMap<String, Object>(); storyResults) throws IOException {
        Map<String, generateStoriesReport(final List<StoryTestResults>
        storyContext.put("stories", storyResults);
           Map<String, Object> context = new HashMap<String, Object>();
        storyContext.put("storyContext", "All stories");
           context.put("stories", storyResults);
        addFormattersToContext(storyContext); stories");
           context.put("storyContext", "All
        writeReportToOutputDirectory("stories.html",
           addFormattersToContext(context);
                                     mergeTemplate(STORIES_TEMPLATE_PATH).usingContext(storyContext));
           String htmlContents = mergeTemplate(STORIES_TEMPLATE_PATH).usingContext(context);
           writeReportToOutputDirectory("stories.html", htmlContents);
        Map<String, Object> featureContext = new HashMap<String, Object>();
      }
       addFormattersToContext(featureContext);
       featureContext.put("features", featureResults);                Refactor into clear steps
       writeReportToOutputDirectory("features.html",
                                    mergeTemplate(FEATURES_TEMPLATE_PATH).usingContext(featureContext));

       for(FeatureResults feature : featureResults) {
           generateStoryReportForFeature(feature);
       }

       generateReportHomePage(storyResults, featureResults);

       getTestHistory().updateData(featureResults);

       generateHistoryReport();
  }
High-Level
Methods	
  should	
  be	
  small                                     Refactoring
  public void generateAggregateReportFor(final List<StoryTestResults> storyResults,
                                         final List<FeatureResults> featureResults) throws IOException {
      LOGGER.info("Generating summary report for user stories to "+ getOutputDirectory());

      copyResourcesToOutputDirectory();

      generateStoriesReportFor(storyResults);

      Map<String, Object> featureContext = new HashMap<String, Object>();
      addFormattersToContext(featureContext);
      featureContext.put("features", featureResults);
      writeReportToOutputDirectory("features.html",
                                   mergeTemplate(FEATURES_TEMPLATE_PATH).usingContext(featureContext));

      for(FeatureResults feature : featureResults) {
          generateStoryReportForFeature(feature);
      }

      generateReportHomePage(storyResults, featureResults);

      getTestHistory().updateData(featureResults);

                   private void updateHistoryFor(final List<FeatureResults> featureResults) {
      generateHistoryReport();
  }                     getTestHistory().updateData(featureResults);
                   }
High-Level
Methods	
  should	
  be	
  small                         Refactoring

private void generateAggregateReportFor(final List<StoryTestResults> storyResults,
                                        final List<FeatureResults> featureResults)
    throws IOException {

        copyResourcesToOutputDirectory();

        generateStoriesReportFor(storyResults);
        generateFeatureReportFor(featureResults);
        generateReportHomePage(storyResults, featureResults);

        updateHistoryFor(featureResults);
        generateHistoryReport();
}
High-Level Refactoring
   Methods	
  should	
  only	
  do	
  one	
  thing

                                                                    Too much going on here...
public String getReportName(String reportType, final String qualifier) {
    if (qualifier == null) {
        String testName = "";
        if (getUserStory() != null) {
             testName = NameConverter.underscore(getUserStory().getName());
        }
        String scenarioName = NameConverter.underscore(getMethodName());
        testName = withNoIssueNumbers(withNoArguments(appendToIfNotNull(testName, scenarioName)));
        return testName + "." + reportType;
    } else {
        String userStory = "";
        if (getUserStory() != null) {
             userStory = NameConverter.underscore(getUserStory().getName()) + "_";
        }
        String normalizedQualifier = qualifier.replaceAll(" ", "_");
        return userStory + withNoArguments(getMethodName()) + "_" + normalizedQualifier + "." + reportType;
    }
}
                                                                    Mixing what and how
High-Level Refactoring
    Methods	
  should	
  only	
  do	
  one	
  thing

                                                               Chose what to do here
public String getReportName(final ReportType type, final String qualifier) {
    ReportNamer reportNamer = ReportNamer.forReportType(type);
    if (shouldAddQualifier(qualifier)) {
        return reportNamer.getQualifiedTestNameFor(this, qualifier);
    } else {
        return reportNamer.getNormalizedTestNameFor(this);
    }
}



                                     The how is the responsibility of another class
Encapsulate	
  boolean	
  expressions
for (TestStep currentStep : testSteps) {
    if (!currentStep.isAGroup() && currentStep.getScreenshots() != null) {
        for (RecordedScreenshot screenshot : currentStep.getScreenshots()) {
            screenshots.add(new Screenshot(screenshot.getScreenshot().getName(),
                    currentStep.getDescription(),
                    widthOf(screenshot.getScreenshot()),
                    currentStep.getException()));
        }
    }
}



                                                 What does this boolean mean?

for (TestStep currentStep : testSteps) {
    if (currentStep.needsScreenshots()) {
        ...
              public boolean needsScreenshots() {
                  return (!isAGroup() && getScreenshotsAndHtmlSources() != null);
              }




                                                   Expresses the intent better
Avoid	
  unclear/ambiguous	
  class	
  name
         for (TestStep currentStep : testSteps) {
             if (currentStep.needsScreenshots()) {
                 for (RecordedScreenshot screenshot : currentStep.getScreenshots()) {
                     screenshots.add(new Screenshot(screenshot.getScreenshot().getName(),
                             currentStep.getDescription(),
                             widthOf(screenshot.getScreenshot()),
                             currentStep.getException()));
                 }
             }
         }
                                                     Is this class name really accurate?

 public List<Screenshot> getScreenshots() {                           Too many screenshots!
     List<Screenshot> screenshots = new ArrayList<Screenshot>();
     List<TestStep> testSteps = getFlattenedTestSteps();

     for (TestStep currentStep : testSteps) {
         if (weNeedAScreenshotFor(currentStep)) {
             for (ScreenshotAndHtmlSource screenshotAndHtml : currentStep.getScreenshotsAndHtmlSources()) {
                 screenshots.add(new Screenshot(screenshotAndHtml.getScreenshotFile().getName(),
                         currentStep.getDescription(),
                         widthOf(screenshotAndHtml.getScreenshot()),
                         currentStep.getException()));
             }
         }
     }
     return ImmutableList.copyOf(screenshots);
Using a more revealing class name
 }
                                                              And a clearer method name
Encapsulate	
  overly-­‐complex	
  code

public List<Screenshot> getScreenshots() {
    List<Screenshot> screenshots = new ArrayList<Screenshot>();
    List<TestStep> testSteps = getFlattenedTestSteps();

    for (TestStep currentStep : testSteps) {
        if (currentStep.needsScreenshots()) {
            for (ScreenshotAndHtmlSource screenshotAndHtml : currentStep.getScreenshotsAndHtmlSources()) {
                screenshots.add(new Screenshot(screenshotAndHtml.getScreenshot().getName(),
                        currentStep.getDescription(),
                        widthOf(screenshotAndHtml.getScreenshot()),
                        currentStep.getException()));
            }
        }
    }
    return ImmutableList.copyOf(screenshots);
                                                 What does all this do?
}
      public List<Screenshot> getScreenshots() {
          List<Screenshot> screenshots = new ArrayList<Screenshot>();
          List<TestStep> testSteps = getFlattenedTestSteps();

          for (TestStep currentStep : testSteps) {
              if (weNeedAScreenshotFor(currentStep)) {
                  screenshots.addAll(
                      convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep)));
              }
          }

      }
          return ImmutableList.copyOf(screenshots);                          Clearer intention
Encapsulate	
  overly-­‐complex	
  code

public List<Screenshot> getScreenshots() {
    List<Screenshot> screenshots = new ArrayList<Screenshot>();
    List<TestStep> testSteps = getFlattenedTestSteps();

      for (TestStep currentStep : testSteps) {
          if (currentStep.needsScreenshots()) {
              screenshots.addAll(
                  convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep)));
          }
      }
      return ImmutableList.copyOf(screenshots);        What we are doing
}
    private Converter<ScreenshotAndHtmlSource, Screenshot> toScreenshotsFor(final TestStep currentStep) {
        return new Converter<ScreenshotAndHtmlSource, Screenshot>() {
            @Override
            public Screenshot convert(ScreenshotAndHtmlSource from) {
                return new Screenshot(from.getScreenshotFile().getName(),
                                      currentStep.getDescription(),
                                      widthOf(from.getScreenshotFile()),
                                      currentStep.getException());
            }
        };
    }

                                                                               How we do it
Avoid	
  deep	
  nes;ng

public List<Screenshot> getScreenshots() {
    List<Screenshot> screenshots = new ArrayList<Screenshot>();
    List<TestStep> testSteps = getFlattenedTestSteps();

        for (TestStep currentStep : testSteps) {
            if (currentStep.needsScreenshots()) {
                                                               Code doing too many things
                screenshots.addAll(
                    convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep)));
            }
        }
        return ImmutableList.copyOf(screenshots);
}                                                                 Break the code down into
    public List<Screenshot> getScreenshots() {                    logical steps
        List<Screenshot> screenshots = new ArrayList<Screenshot>();

         List<TestStep> testStepsWithScreenshots = select(getFlattenedTestSteps(),
                                                          having(on(TestStep.class).needsScreenshots()));

         for (TestStep currentStep : testStepsWithScreenshots) {
             screenshots.addAll(convert(currentStep.getScreenshotsAndHtmlSources(),
                                        toScreenshotsFor(currentStep)));
         }

         return ImmutableList.copyOf(screenshots);
    }                                                     Remove the nested condition
Keep	
  each	
  step	
  simple!
public List<Screenshot> getScreenshots() {
    List<Screenshot> screenshots = new ArrayList<Screenshot>();

        List<TestStep> testStepsWithScreenshots = select(getFlattenedTestSteps(),
                                                         having(on(TestStep.class).needsScreenshots()));

        for (TestStep currentStep : testStepsWithScreenshots) {
            screenshots.addAll(convert(currentStep.getScreenshotsAndHtmlSources(),
                                       toScreenshotsFor(currentStep)));
        }

        return ImmutableList.copyOf(screenshots);           Too much happening here?
}

    public List<Screenshot> getScreenshots() {
        List<Screenshot> screenshots = new ArrayList<Screenshot>();

          List<TestStep> testStepsWithScreenshots = select(getFlattenedTestSteps(),
                                                           having(on(TestStep.class).needsScreenshots()));

          for (TestStep currentStep : testStepsWithScreenshots) {
              screenshots.addAll(screenshotsIn(currentStep));
          }

          return ImmutableList.copyOf(screenshots);       This reads more smoothly
    }

    private List<Screenshot> screenshotsIn(TestStep currentStep) {
        return convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep));
    }
Code	
  should	
  communicate	
  fluently
Use	
  Fluent	
  APIs


               Complex domain object
                             Lots of variants
                                  Object tree



       FundsTransferOrder order = new FundsTransferOrder();
       order.setType("SWIFT");
       Party originatorParty = organizationServer.findPartyByCode("WPAC");
       order.setOriginatorParty(originatorParty);
       Party counterParty = organizationServer.findPartyByCode("CBAA");
       order.setCounterParty(counterParty);
       order.setDate(DateTime.parse("22/11/2011"));
       Currency currency = currencyTable.findCurrency("USD");
       Amount amount = new Amount(500, currency);
       order.setAmount(amount);
Complex code
       ...


  Need to know how to create the child objects
Use	
  Fluent	
  APIs




                                           More readable
                                               No object creation
                                                   Easier to maintain
FundsTransferOrder.createNewSWIFTOrder()
                                  .fromOriginatorParty("WPAC")
                                  .toCounterParty("CBAA")
                                  .forDebitor("WPAC")
                                  .and().forCreditor("CBAA")
                                  .settledOnThe("22/11/2011")
                                  .forAnAmountOf(500, US_DOLLARS)
                                  .asXML();
Use	
  Fluent	
  APIs


                                         Readable parameter style


TestStatistics testStatistics = testStatisticsProvider.statisticsForTests(With.tag("Boat sales"));

double recentBoatSalePassRate = testStatistics.getPassRate().overTheLast(5).testRuns();




                   Fluent method call
Use	
  Fluent	
  APIs
                             Fluent style...


                        A builder does the
                            dirty work

                           Represents how
                            to select steps

                                Override to
                               select different
                                 step types
Your	
  code	
  is	
  organic




     Help	
  it	
  grow
Replace	
  Constructors	
  with	
  Crea7on	
  Methods




                             Too many constructors
                                Business knowledge
                                hidden in the constructors
                                  Which one should I use?
Replace	
  Constructors	
  with	
  Crea7on	
  Methods




                                   Private constructor




        Static creator methods
          One implementing class
Replace	
  Constructors	
  with	
  Crea7on	
  Methods




                  Communicates	
  the	
  intended	
  use	
  be5er
                  Overcomes	
  technical	
  limits	
  with	
  constructors
                  Inconsistent	
  object	
  crea<on	
  pa5erns
Encapsulate	
  Classes	
  with	
  a	
  Factory
                     High-Level Refactoring
                                           Only this interface should
                                           be visible




Many different
implementations
                          Which implementation
                          should I use?
Encapsulate	
  Classes	
  with	
  a	
  Factory
                                 High-Level Refactoring
                             Helpful factory methods




Easier	
  to	
  create	
  the	
  right	
  instances
Hides	
  classes	
  that	
  don’t	
  need	
  to	
  be	
  exposed
Encourages	
  “programming	
  to	
  an	
  interface”
Need	
  a	
  new	
  method	
  when	
  new	
  classes	
  are	
  added
Need	
  access	
  to	
  factory	
  class	
  to	
  customize/extend
Plenty	
  of	
  other	
  refactoring	
  pa2erns...




                    But	
  know	
  why	
  you	
  are	
  applying	
  them
Learn	
  about	
  Func7onal	
  Programming!
Func;ons	
  as	
  a	
  1st	
  class	
  concept
Immutable	
  value	
  objects
No	
  side	
  effects
Benefit	
  from	
  concurrency
No	
  loops
5050



          A, B, C, D, F



          Parallel processing




       TRUE
Functional programming in Java?




Surely this is
 madness!
lambdaj
– DSL for manipulating collections in a functional style
– Replace loops with more concise and readable code




Functional constructs in Java
LambdaJ support many high level
collection-related functions


 filter                   aggregate


      sort
                       convert


extract        index             group
Find all adult ages                                    filter

            List<Integer> adultAges = new ArrayList<Integer>();
            for(int age : ages) {
                if (age >= 18) {
                    adults.add(age);
                }
            }




List<Integer> adultAges = filter(greaterThanOrEqualTo(18), ages);
Find all adults                                           filter

                List<Person> adults = new ArrayList<Person>();
                for(int person : people) {
                    if (person.getAge() >= 18) {
                        adults.add(person);
                    }
                }




List<Person> adults = filter(having(on(Person.class).getAge(), greaterThanOrEqualTo(18)),
                             people);
Find all the sales of Ferraris                                filter

            List<Sale> salesOfAFerrari = new ArrayList<Sale>();
            for (Sale sale : sales) {
                if (sale.getCar().getBrand().equals("Ferrari"))
                    salesOfAFerrari.add(sale);
            }




List<Sale> salesOfAFerrari = select(sales,
    having(on(Sale.class).getCar().getBrand(),equalTo("Ferrari")));
Sort sales by cost                                       sort

     List<Sale> sortedSales = new ArrayList<Sale>(sales);
     Collections.sort(sortedSales, new Comparator<Sale>() {
         public int compare(Sale s1, Sale s2) {
             return Double.valueOf(s1.getCost()).compareTo(s2.getCost());
         }
     });




List<Sale> sortedSales = sort(sales, on(Sale.class).getCost());
Index cars by brand                                   index


       Map<String, Car> carsByBrand = new HashMap<String, Car>();
       for (Car car : db.getCars()) {
           carsByBrand.put(car.getBrand(), car);
       }




Map<String, Car> carsByBrand = index(cars, on(Car.class).getBrand());
Find the total sales
                                                  aggregate

  double totalSales = 0.0;
  for (Sale sale : sales) {
      totalSales = totalSales + sale.getCost();
  }




      double totalSales = sumFrom(sales).getCost();
Find most costly sale
                                             aggregate

   double maxCost = 0.0;
   for (Sale sale : sales) {
       double cost = sale.getCost();
       if (cost > maxCost) maxCost = cost;
   }




          double maxCost = maxFrom(sales).getCost();
extract
Extract the costs of the cars as a list




      List<Double> costs = new ArrayList<Double>();
      for (Car car : cars) costs.add(car.getCost());




  List<Double> costs = extract(cars, on(Car.class).getCost());
guava immutable collections
guava immutable collections
   Defensive
   Thread-safe
   Efficient
Creating an immutable list

List<String> colors = ImmutableList.of("red", "green", "blue");




  ImmutableSet<Field> fields =
                     ImmutableSet.copyOf(stepLibraryClass.getDeclaredFields());



                                             Creating an immutable copy
The	
  Boy	
  Scout	
  Rule




   “Leave	
  the	
  camp	
  ground	
  cleaner	
  than	
  you	
  found	
  it”
Clean Coding Practices for Java Developers




                Thank You



                                    John	
  Ferguson	
  Smart
                             john.smart@wakaleo.com
                             	
  h2p://www.wakaleo.com
                                        Twi2er:	
  wakaleo

Clean coding-practices

  • 1.
    Clean Coding Practicesfor Java Developers Clean Coding Practices for Java Developers John  Ferguson  Smart john.smart@wakaleo.com  h2p://www.wakaleo.com Twi2er:  wakaleo
  • 2.
    So who isthis guy, anyway? Consulta nt Trainer Mentor Author Speaker Coder John Fer guson S mar t
  • 3.
    Who  are  you  wri;ng  code  for,  anyway?
  • 4.
    Who  are  you  wri;ng  code  for,  anyway? The  computer?
  • 5.
    Who  are  you  wri;ng  code  for,  anyway? Other  developers?
  • 6.
    Who  are  you  wri;ng  code  for,  anyway? Your  next  boss?
  • 7.
    Clean Coding Practicesfor Java Developers Why  is  clean  code  so  important?
  • 8.
    Easier  to  Understand Easier  to  Change Cheaper  to  Maintain
  • 9.
    Choose  your  names  well "What's in a name? That which we call a rose By any other name would smell as sweet." Romeo and Juliet (II, ii, 1-2) Really?
  • 10.
    Use  a  common  vocabulary getCustomerInfo() getClientDetails() getCustomerRecord() ... Which one do I use? Are they the same? getCustomer() Choose one and stick to it
  • 11.
    Use  meaningful  names intdays; What does this represent? int daysSinceCreation; int daysSinceLastModification; int durationInDays; These are all more meaningful choices
  • 12.
    Don’t  talk  in  code class DtaRcrd102 { private Date genymdhms; private Date modymdhms; private final String pszqint = “102”; ... } Huh? class Customer { private Date generationTimestamp; private Date modificationTimestamp; private final String recordId = “102” ... }
  • 13.
    But  don’t  state  the  obvious List<Client> clientList; Is ‘List’ significant or just noise? List<Client> clients; List<Client> regularClients; List<Client> newClients;
  • 14.
    In  short:  Don’t  make  me  think! What does this do? Clearer parameter name More explicit method name What are we counting? Method call rather than boolean expression
  • 15.
    Make  your  code  tell  a  story
  • 16.
    High-Level Methods  should  be  small Refactoring public void generateAggregateReportFor(final List<StoryTestResults> storyResults, final List<FeatureResults> featureResults) throws IOException { LOGGER.info("Generating summary report for user stories to "+ getOutputDirectory()); copyResourcesToOutputDirectory(); Map<String, Object> storyContext = new HashMap<String, Object>(); storyContext.put("stories", storyResults); storyContext.put("storyContext", "All stories"); addFormattersToContext(storyContext); writeReportToOutputDirectory("stories.html", mergeTemplate(STORIES_TEMPLATE_PATH).usingContext(storyContext)); Map<String, Object> featureContext = new HashMap<String, Object>(); addFormattersToContext(featureContext); featureContext.put("features", featureResults); writeReportToOutputDirectory("features.html", mergeTemplate(FEATURES_TEMPLATE_PATH).usingContext(featureContext)); for(FeatureResults feature : featureResults) { generateStoryReportForFeature(feature); } generateReportHomePage(storyResults, featureResults); Code hard to understand getTestHistory().updateData(featureResults); generateHistoryReport(); }
  • 17.
    High-Level Methods  should  be  small Refactoring public void generateAggregateReportFor(final List<StoryTestResults> storyResults, final List<FeatureResults> featureResults) throws IOException { LOGGER.info("Generating summary report for user stories to "+ getOutputDirectory()); copyResourcesToOutputDirectory(); private void Object> storyContext = new HashMap<String, Object>(); storyResults) throws IOException { Map<String, generateStoriesReport(final List<StoryTestResults> storyContext.put("stories", storyResults); Map<String, Object> context = new HashMap<String, Object>(); storyContext.put("storyContext", "All stories"); context.put("stories", storyResults); addFormattersToContext(storyContext); stories"); context.put("storyContext", "All writeReportToOutputDirectory("stories.html", addFormattersToContext(context); mergeTemplate(STORIES_TEMPLATE_PATH).usingContext(storyContext)); String htmlContents = mergeTemplate(STORIES_TEMPLATE_PATH).usingContext(context); writeReportToOutputDirectory("stories.html", htmlContents); Map<String, Object> featureContext = new HashMap<String, Object>(); } addFormattersToContext(featureContext); featureContext.put("features", featureResults); Refactor into clear steps writeReportToOutputDirectory("features.html", mergeTemplate(FEATURES_TEMPLATE_PATH).usingContext(featureContext)); for(FeatureResults feature : featureResults) { generateStoryReportForFeature(feature); } generateReportHomePage(storyResults, featureResults); getTestHistory().updateData(featureResults); generateHistoryReport(); }
  • 18.
    High-Level Methods  should  be  small Refactoring public void generateAggregateReportFor(final List<StoryTestResults> storyResults, final List<FeatureResults> featureResults) throws IOException { LOGGER.info("Generating summary report for user stories to "+ getOutputDirectory()); copyResourcesToOutputDirectory(); generateStoriesReportFor(storyResults); Map<String, Object> featureContext = new HashMap<String, Object>(); addFormattersToContext(featureContext); featureContext.put("features", featureResults); writeReportToOutputDirectory("features.html", mergeTemplate(FEATURES_TEMPLATE_PATH).usingContext(featureContext)); for(FeatureResults feature : featureResults) { generateStoryReportForFeature(feature); } generateReportHomePage(storyResults, featureResults); getTestHistory().updateData(featureResults); private void updateHistoryFor(final List<FeatureResults> featureResults) { generateHistoryReport(); } getTestHistory().updateData(featureResults); }
  • 19.
    High-Level Methods  should  be  small Refactoring private void generateAggregateReportFor(final List<StoryTestResults> storyResults, final List<FeatureResults> featureResults) throws IOException { copyResourcesToOutputDirectory(); generateStoriesReportFor(storyResults); generateFeatureReportFor(featureResults); generateReportHomePage(storyResults, featureResults); updateHistoryFor(featureResults); generateHistoryReport(); }
  • 20.
    High-Level Refactoring Methods  should  only  do  one  thing Too much going on here... public String getReportName(String reportType, final String qualifier) { if (qualifier == null) { String testName = ""; if (getUserStory() != null) { testName = NameConverter.underscore(getUserStory().getName()); } String scenarioName = NameConverter.underscore(getMethodName()); testName = withNoIssueNumbers(withNoArguments(appendToIfNotNull(testName, scenarioName))); return testName + "." + reportType; } else { String userStory = ""; if (getUserStory() != null) { userStory = NameConverter.underscore(getUserStory().getName()) + "_"; } String normalizedQualifier = qualifier.replaceAll(" ", "_"); return userStory + withNoArguments(getMethodName()) + "_" + normalizedQualifier + "." + reportType; } } Mixing what and how
  • 21.
    High-Level Refactoring Methods  should  only  do  one  thing Chose what to do here public String getReportName(final ReportType type, final String qualifier) { ReportNamer reportNamer = ReportNamer.forReportType(type); if (shouldAddQualifier(qualifier)) { return reportNamer.getQualifiedTestNameFor(this, qualifier); } else { return reportNamer.getNormalizedTestNameFor(this); } } The how is the responsibility of another class
  • 22.
    Encapsulate  boolean  expressions for(TestStep currentStep : testSteps) { if (!currentStep.isAGroup() && currentStep.getScreenshots() != null) { for (RecordedScreenshot screenshot : currentStep.getScreenshots()) { screenshots.add(new Screenshot(screenshot.getScreenshot().getName(), currentStep.getDescription(), widthOf(screenshot.getScreenshot()), currentStep.getException())); } } } What does this boolean mean? for (TestStep currentStep : testSteps) { if (currentStep.needsScreenshots()) { ... public boolean needsScreenshots() { return (!isAGroup() && getScreenshotsAndHtmlSources() != null); } Expresses the intent better
  • 23.
    Avoid  unclear/ambiguous  class  name for (TestStep currentStep : testSteps) { if (currentStep.needsScreenshots()) { for (RecordedScreenshot screenshot : currentStep.getScreenshots()) { screenshots.add(new Screenshot(screenshot.getScreenshot().getName(), currentStep.getDescription(), widthOf(screenshot.getScreenshot()), currentStep.getException())); } } } Is this class name really accurate? public List<Screenshot> getScreenshots() { Too many screenshots! List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testSteps = getFlattenedTestSteps(); for (TestStep currentStep : testSteps) { if (weNeedAScreenshotFor(currentStep)) { for (ScreenshotAndHtmlSource screenshotAndHtml : currentStep.getScreenshotsAndHtmlSources()) { screenshots.add(new Screenshot(screenshotAndHtml.getScreenshotFile().getName(), currentStep.getDescription(), widthOf(screenshotAndHtml.getScreenshot()), currentStep.getException())); } } } return ImmutableList.copyOf(screenshots); Using a more revealing class name } And a clearer method name
  • 24.
    Encapsulate  overly-­‐complex  code publicList<Screenshot> getScreenshots() { List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testSteps = getFlattenedTestSteps(); for (TestStep currentStep : testSteps) { if (currentStep.needsScreenshots()) { for (ScreenshotAndHtmlSource screenshotAndHtml : currentStep.getScreenshotsAndHtmlSources()) { screenshots.add(new Screenshot(screenshotAndHtml.getScreenshot().getName(), currentStep.getDescription(), widthOf(screenshotAndHtml.getScreenshot()), currentStep.getException())); } } } return ImmutableList.copyOf(screenshots); What does all this do? } public List<Screenshot> getScreenshots() { List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testSteps = getFlattenedTestSteps(); for (TestStep currentStep : testSteps) { if (weNeedAScreenshotFor(currentStep)) { screenshots.addAll( convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep))); } } } return ImmutableList.copyOf(screenshots); Clearer intention
  • 25.
    Encapsulate  overly-­‐complex  code publicList<Screenshot> getScreenshots() { List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testSteps = getFlattenedTestSteps(); for (TestStep currentStep : testSteps) { if (currentStep.needsScreenshots()) { screenshots.addAll( convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep))); } } return ImmutableList.copyOf(screenshots); What we are doing } private Converter<ScreenshotAndHtmlSource, Screenshot> toScreenshotsFor(final TestStep currentStep) { return new Converter<ScreenshotAndHtmlSource, Screenshot>() { @Override public Screenshot convert(ScreenshotAndHtmlSource from) { return new Screenshot(from.getScreenshotFile().getName(), currentStep.getDescription(), widthOf(from.getScreenshotFile()), currentStep.getException()); } }; } How we do it
  • 26.
    Avoid  deep  nes;ng publicList<Screenshot> getScreenshots() { List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testSteps = getFlattenedTestSteps(); for (TestStep currentStep : testSteps) { if (currentStep.needsScreenshots()) { Code doing too many things screenshots.addAll( convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep))); } } return ImmutableList.copyOf(screenshots); } Break the code down into public List<Screenshot> getScreenshots() { logical steps List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testStepsWithScreenshots = select(getFlattenedTestSteps(), having(on(TestStep.class).needsScreenshots())); for (TestStep currentStep : testStepsWithScreenshots) { screenshots.addAll(convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep))); } return ImmutableList.copyOf(screenshots); } Remove the nested condition
  • 27.
    Keep  each  step  simple! public List<Screenshot> getScreenshots() { List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testStepsWithScreenshots = select(getFlattenedTestSteps(), having(on(TestStep.class).needsScreenshots())); for (TestStep currentStep : testStepsWithScreenshots) { screenshots.addAll(convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep))); } return ImmutableList.copyOf(screenshots); Too much happening here? } public List<Screenshot> getScreenshots() { List<Screenshot> screenshots = new ArrayList<Screenshot>(); List<TestStep> testStepsWithScreenshots = select(getFlattenedTestSteps(), having(on(TestStep.class).needsScreenshots())); for (TestStep currentStep : testStepsWithScreenshots) { screenshots.addAll(screenshotsIn(currentStep)); } return ImmutableList.copyOf(screenshots); This reads more smoothly } private List<Screenshot> screenshotsIn(TestStep currentStep) { return convert(currentStep.getScreenshotsAndHtmlSources(), toScreenshotsFor(currentStep)); }
  • 28.
  • 29.
    Use  Fluent  APIs Complex domain object Lots of variants Object tree FundsTransferOrder order = new FundsTransferOrder(); order.setType("SWIFT"); Party originatorParty = organizationServer.findPartyByCode("WPAC"); order.setOriginatorParty(originatorParty); Party counterParty = organizationServer.findPartyByCode("CBAA"); order.setCounterParty(counterParty); order.setDate(DateTime.parse("22/11/2011")); Currency currency = currencyTable.findCurrency("USD"); Amount amount = new Amount(500, currency); order.setAmount(amount); Complex code ... Need to know how to create the child objects
  • 30.
    Use  Fluent  APIs More readable No object creation Easier to maintain FundsTransferOrder.createNewSWIFTOrder() .fromOriginatorParty("WPAC") .toCounterParty("CBAA") .forDebitor("WPAC") .and().forCreditor("CBAA") .settledOnThe("22/11/2011") .forAnAmountOf(500, US_DOLLARS) .asXML();
  • 31.
    Use  Fluent  APIs Readable parameter style TestStatistics testStatistics = testStatisticsProvider.statisticsForTests(With.tag("Boat sales")); double recentBoatSalePassRate = testStatistics.getPassRate().overTheLast(5).testRuns(); Fluent method call
  • 32.
    Use  Fluent  APIs Fluent style... A builder does the dirty work Represents how to select steps Override to select different step types
  • 33.
    Your  code  is  organic Help  it  grow
  • 34.
    Replace  Constructors  with  Crea7on  Methods Too many constructors Business knowledge hidden in the constructors Which one should I use?
  • 35.
    Replace  Constructors  with  Crea7on  Methods Private constructor Static creator methods One implementing class
  • 36.
    Replace  Constructors  with  Crea7on  Methods Communicates  the  intended  use  be5er Overcomes  technical  limits  with  constructors Inconsistent  object  crea<on  pa5erns
  • 37.
    Encapsulate  Classes  with  a  Factory High-Level Refactoring Only this interface should be visible Many different implementations Which implementation should I use?
  • 38.
    Encapsulate  Classes  with  a  Factory High-Level Refactoring Helpful factory methods Easier  to  create  the  right  instances Hides  classes  that  don’t  need  to  be  exposed Encourages  “programming  to  an  interface” Need  a  new  method  when  new  classes  are  added Need  access  to  factory  class  to  customize/extend
  • 39.
    Plenty  of  other  refactoring  pa2erns... But  know  why  you  are  applying  them
  • 40.
  • 41.
    Func;ons  as  a  1st  class  concept
  • 42.
  • 43.
  • 44.
  • 45.
  • 46.
    5050 A, B, C, D, F Parallel processing TRUE
  • 47.
    Functional programming inJava? Surely this is madness!
  • 48.
    lambdaj – DSL formanipulating collections in a functional style – Replace loops with more concise and readable code Functional constructs in Java
  • 49.
    LambdaJ support manyhigh level collection-related functions filter aggregate sort convert extract index group
  • 50.
    Find all adultages filter List<Integer> adultAges = new ArrayList<Integer>(); for(int age : ages) { if (age >= 18) { adults.add(age); } } List<Integer> adultAges = filter(greaterThanOrEqualTo(18), ages);
  • 51.
    Find all adults filter List<Person> adults = new ArrayList<Person>(); for(int person : people) { if (person.getAge() >= 18) { adults.add(person); } } List<Person> adults = filter(having(on(Person.class).getAge(), greaterThanOrEqualTo(18)), people);
  • 52.
    Find all thesales of Ferraris filter List<Sale> salesOfAFerrari = new ArrayList<Sale>(); for (Sale sale : sales) {     if (sale.getCar().getBrand().equals("Ferrari"))         salesOfAFerrari.add(sale); } List<Sale> salesOfAFerrari = select(sales, having(on(Sale.class).getCar().getBrand(),equalTo("Ferrari")));
  • 53.
    Sort sales bycost sort List<Sale> sortedSales = new ArrayList<Sale>(sales); Collections.sort(sortedSales, new Comparator<Sale>() {     public int compare(Sale s1, Sale s2) {         return Double.valueOf(s1.getCost()).compareTo(s2.getCost());     } }); List<Sale> sortedSales = sort(sales, on(Sale.class).getCost());
  • 54.
    Index cars bybrand index Map<String, Car> carsByBrand = new HashMap<String, Car>(); for (Car car : db.getCars()) {     carsByBrand.put(car.getBrand(), car); } Map<String, Car> carsByBrand = index(cars, on(Car.class).getBrand());
  • 55.
    Find the totalsales aggregate double totalSales = 0.0; for (Sale sale : sales) {     totalSales = totalSales + sale.getCost(); } double totalSales = sumFrom(sales).getCost();
  • 56.
    Find most costlysale aggregate double maxCost = 0.0; for (Sale sale : sales) {     double cost = sale.getCost();     if (cost > maxCost) maxCost = cost; } double maxCost = maxFrom(sales).getCost();
  • 57.
    extract Extract the costsof the cars as a list List<Double> costs = new ArrayList<Double>(); for (Car car : cars) costs.add(car.getCost()); List<Double> costs = extract(cars, on(Car.class).getCost());
  • 58.
  • 59.
    guava immutable collections Defensive Thread-safe Efficient
  • 60.
    Creating an immutablelist List<String> colors = ImmutableList.of("red", "green", "blue"); ImmutableSet<Field> fields = ImmutableSet.copyOf(stepLibraryClass.getDeclaredFields()); Creating an immutable copy
  • 61.
    The  Boy  Scout  Rule “Leave  the  camp  ground  cleaner  than  you  found  it”
  • 62.
    Clean Coding Practicesfor Java Developers Thank You John  Ferguson  Smart john.smart@wakaleo.com  h2p://www.wakaleo.com Twi2er:  wakaleo