Clean Code

Alexis Pavlidis

Full Stack Developer at Intrasoft International S.A.

Software Engineer

& Aspiring Craftsman

What is Clean Code?

  • Readable
  • Maintanable
  • Testable
  • Elegant

You know you are working on clean code when each
routine you read turns out to be pretty much what
you expected.

Ward Cunningham

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

VS

Meaningful Names

Intention revealing names
class DtaRcrd102 {
    private Date genymdhms;
    private modymdhms;
    /* .... */
}
class Customer {
    private Date generationTimestamp;
    private Date modificationTimestamp;
    /* .... */
}

VS

Meaningful Names

Pronounceable names
ClassifierNumber classifierString; //name not changed when type changed!!
public interface ProcessI {

    default void print(){ 
        System.out.println("Item processed"); 
    } 

}

Meaningful Names

Hungarian notation
CustomBigDecimal totalAmount = new CustomBigDecimal(250.0);
CustomBigDecimal totalAmount = CustomBigDecimal.fromRealNumber(250.0);

VS

Meaningful Names

Method names
  Student student = new Student.Builder(1L)
                .firstName("Alexis")
                .lastName("Pavlidis")
                .semester(8)
                .address("20 Georgiou 8")
                .email("apavlidi@it.teithe.gr")
                .phoneNumber(6999999L)
                .registeredDate(LocalDate.of(1996, 10, 9)).build();
 Student student = new Student(1L,"Alexis","Pavlidis",8,
                                "20 Georgiou 8","apavlidi@it.teithe.gr",
                                6999999L,LocalDate.of(1996, 10, 9))

VS

Meaningful Names

Method names Pt.2
public String add(String firstName, String lastName){ 
    return firstName + " : " + lastName;
}


public void add(ProductItem item){       
    this.productItems.add(item);
}
public void add(String firstName, String lastName){ 
    return firstName + " : " + lastName;
}


public void insert(ProductItem item){       
    this.productItems.add(item);
}

VS

Meaningful Names

Don`t pun
getActiveAccount();
getActiveAccountInfo();
BigDecimal amount;
BigDecimal amountMoney;

Meaningful Names

Make meaningful distinctions
public class CustomerInfo {
    /* .... */
}

public class CustomerData {
    /* .... */
} 

Rules of functions:

  1. should be small
  2. should be smaller than that
  3. <150 characters per line
  4. <20 lines

 

Do one thing

FUNCTIONS SHOULD DO ONE THING ONLY

 

 

Functions

public void emailClients(List<Client> clients) {
    for (Client client : clients) {
        Client clientRecord = repository.findOne(client.getId());
        if (clientRecord.isActive()){
            email(client);
        }
    }
}
public void emailClients(List<Client> clients) {
    for (Client client : clients) {
        if (isActiveClient(client)) {
            email(client);
        }
    }
}

private boolean isActiveClient(Client client) {
    Client clientRecord = repository.findOne(client.getId());
    return clientRecord.isActive();
}

VS

Functions

Do one thing!
public void DoStuff(string email, string userName, string country, string city, string street, string complement, string phone)
{
    /* .... */
}
public void DoStuff(User user, Address address)
{
   /* .... */
}

VS

Functions

Function arguments: Objects
static int calculateInvoices(int multiplier, BigDecimal... invoices) {
    int totalBalance = 0;
    for (int invoice : invoices){
        totalBalance += invoice * multiplier;
    }
    return totalBalance ;
}

Functions

Function arguments : Lists
public Booking book(Customer aCustomer, boolean isPremium) {
      if(isPremium) 
       // logic for premium book
      else
       // logic for regular booking
    }
public class Concert {
      public Booking regularBook(Customer aCustomer) {
       // logic for regular book
      }

      public Booking premiumBook(Customer aCustomer) {
       // logic for regular book
      }
}

VS

Functions

Flag arguments
public class UserValidator {
    private Cryptographer cryptographer;
    
    public boolean checkPassword(String userName, String password) {
        User user = UserGateway.findByName(userName);
        if (user != User.NULL) {
            String codedPhrase = user.getPhraseEncodedByPassword();
            String phrase = cryptographer.decrypt(codedPhrase, password);
            if ("Valid Password".equals(phrase)) {
                Session.initialize();
                return true;
            }
        }
        return false;
    }
}

Functions

Have no side effects

Comments

 

If you need comments to understand what the code is doing then the code is poorly written

/**
 MIT License

Copyright (c) 2018 Alexis Pavlidis

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
© 2019 GitHub, Inc.
 */

Comments

Legal comments (Good)
// Don't run this if you're short on time!
public void testFullRequestCycle(){
 
}

// This will overwrite production data...
public function seedDatabase(){
 
}

Comments

Warning comments (Good)
public static void hireEmployee(Employee employee) {

    //TODO Validate employee before persisting to DB

    EmployeeDAO employeeDAO = new EmployeeDAO();
    employeeDAO.saveEmployee(employee);
}

Comments

TODO/FIXME comments (Good)
/**
* Zaps the machine with the number of volts you specify.
* <p>
* Do not exceed more than 30 volts or the zap function will backfire.
*
* @exception IOException if you don't enter a data type amount for the voltage
* @param voltage the number of volts you want to send into the machine
* @see Dynamite#blowDynamite
*/
public void zapMachine(int voltage) throws IOException {
   if (voltage < MAXIMUM_VOLTAGE) {
       System.out.println("Zapping machine with " + voltage + " volts!!!!");
   } else {
    System.out.println("Backfire!!! zapping machine with 1,000,000 volts!!!!");
   }
}

Comments

Javadocs in Public APIs (Good)

Comments

Use variables/functions instead of comments (Bad)
// if the user is not the author of the announcement or if he is not an admin 
// he can`t edit the announcement so throw an exception
if (!(announcement.getPublisher().getId() === req.getUser().getId() 
    || req.getUser().getEduPersonScopedAffiliation() === config.PERMISSIONS.admin)){
    throw new AuthorizationException("No permission to edit the announcement!");
}
if (userIsNotThePublisher(announcement, req.getUser()) && userIsNotAnAdmin(req.getUser()){
    throw new AuthorizationException("No permission to edit the announcement!");
}

private boolean userIsNotThePublisher(Announcement announcement, User user){
    return !announcement.getPublisher().getId().equals(user.getUser().getId());
}

private boolean userIsNotThePublisher(User user){
    return !req.getUser().getEduPersonScopedAffiliation().equals(config.PERMISSIONS.admin));
}

VS

private int bonus(String allRoles, String currentRole, int index) {
    int nextRollScore = scoreForRoll(getNextRoll(allRoles, index));
    if (isLastFrame(allRoles, index))
        return 0;
//    if (currentRole.equals(SPARE_SYMBOL))
//        return nextRollScore;
    if (currentRole.equals(STRIKE_SYMBOL)) {
        String nextRoleString = getNextRoll(allRoles, index + 1);
        return nextRollScore + scoreForRoll(nextRoleString) - 
                               spareDiff(allRoles, nextRoleString, index + 2);
    }
    return 0;
}

Comments

Comment out code (Bad)
  • Source file should be like a newspaper

  • High level concepts on top

  • Details should increase as we move downward

Formatting

Newspaper Metaphor
public class WikiPage {

    private PageData pageData;
    private PageCrawler crawler;
    private List<Article> articles;
    
    /* .... */

    public Item pop(){
        // logic here
    }

    public void add(){
        // logic here
    }

     public void print(){
        // logic here
    }

    /* .... */
    
}

Formatting

Conceptual affinity
public class CustomerValidator {
    
    public boolean checkPassword(String userName, String password){
       Optional<User> user = userRepository.findByUsername(userName);
       if(user.isPresent()){
            return isPasswordValid(password);
        }
        return false;
    }

    private boolean isPasswordValid(String password){
        return checkIfPasswordFollowsRules(password) && 
                   checkIfPasswordIsReused(password);
    }

    private boolean checkIfPasswordFollowsRules(String password){
        // logic here
    }

    private boolean checkIfPasswordIsReused(String password){
        // logic here
    }

}

Formatting

Dependent functions

Formatting

Team rules

 

  • Programmers have personal preferences
  • No code ownership
  • Good software has consistent style
  • Good software reads nicely

Thank you

Any Questions?

Made with Slides.com