Clean-Code

Robert C. Martin aka Uncle Bob

Clean-Code

"Any fool can write code that a computer can understand. Good programmers write code that humans can understand."
Martin Fowler

"Mal nommer les choses, c'est ajouter au malheur du monde !"
Albert Camus

"There are only two hard things in Computer Science:
- cache invalidation,
- naming things" Phil Karlton

Vous allez passer beaucoup plus de temps à relire du code qu'à en écrire.

Namming

Utilisez du vocabulaire métier

    public List<int[]> getFlg() {
      List<int[]> list1 = new ArrayList<int[]>();
      for (int[] x : theList ) {
        if (x[0] == 4)
          list1.add(x);
      }

      return list1;
    }
                        

    public List<Cell> getFlaggedCells() {
      List<Cell> flaggedCells = new ArrayList<Cell>();
      for (Cell cell : gameBoard ) {
        if (cell.isFlagged())
          flaggedCells.add(cell);
      }

      return flaggedCells;
    }
                        

Parlez au business ! en théorie, un PO ou un utilisateur devrait pouvoir lire votre code.
Ou au moins les tests et titres de fonctions.

Bonus : en parlant au business, vous comprendrez vraiment le problème métier.

Evitez les x, i, j, temp

Evitez les List, array, les prefixes de types (u8_price)

Evitez le vocabulaire technique, préferez le vocabulaire métier.

Class : Nom

Methode : verbe d'action

Fonction / Méthode

Small

1 seul niveau d'abstraction

taille de la main ?

pas plus d'un if / for ?

Séparez les niveaux d'abstraction

    public RGBColor readCurrentColor(BlinkLed led) {
        device.sendCommand(new ReadColorRequest(led));

        byte[] response = device.readResponse();

        int red = response[2] >= 0 ? response[2] : response[2] + 256;
        int green = response[3] >= 0 ? response[3] : response[3] + 256;
        int blue = response[4] >= 0 ? response[4] : response[4] + 256;
        return new RGBColor(red, green, blue);
    }
                        

    public RGBColor readCurrentColor(BlinkLed led) {
        device.sendCommand(new ReadColorRequest(led));
        byte[] response = device.readResponse();
        return extractColor(response);
    }
    private RGBColor extractColor(byte[] response) {
        int red = convertToPositiveInt(response[2]);
        int green = convertToPositiveInt(response[3]);
        int blue = convertToPositiveInt(response[4]);
        return new RGBColor(red, green, blue);
    }
    private int convertToPositiveInt(byte byt) {
        return byt >= 0 ? byt : byt + 256;
    }
    

Séparez les niveaux d'abstraction

Extract Method

• Soit une commande : doSomething()
• Soit une requete : getThing()

mais pas les deux !

Commentaires

Inutiles ou Faux

    /*
    * A comment to please checkstyle
    */

    /*
    * Set the port
    *
    * @params port
    */
    public void setPort(Port port) {this.port=port}
    

    ...
            } /* end for */
            dao.flush();
          default :
            break;
          } /* end switch */
        } /* end if */
      } /* end if */
    } catch ...
    

• Soit obsolètes et faux
• Soit redondant et inutiles

Don’t use a comment when you can use a function or a variable

Objets

Abstraction

Expose comment on l'utilise, le métier
pas l'implémentation spécifique

Penser au TDD

Loi de Demeter

Pas bien !

    a.getB().getC().doThings();
                    

    a.doThings();
                    

Un module n'a pas à connaitre l'implémentation des objets qu'il manipule.

Solid

• S - Single-responsiblity principle
• O - Open-closed principle
• L - Liskov substitution principle
• I - Interface segregation principle
• D - Dependency Inversion Principle

S - Single-responsiblity principle

A class should have one and only one reason to change, meaning that a class should have only one job.

Evitez les Managers
Un manager fait un peu de tout mais mal, beaucoup de rien et embrouille tout le monde.

O - Open-closed principle

Objects or entities should be open for extension, but closed for modification.

Penser héritage

Switch, if -> Polymorphisme

public Money calculatePay(Employee e) {
    switch (e.type) {
        case COMMISSIONED:
            return calculateCommissionedPay(e);
            case HOURLY:
                return calculateHourlyPay(e);
            case SALARIED:
                return calculateSalariedPay(e);
            default:
                throw new InvalidEmployeeType(e.type);
        }
    }
                

L - Liskov substitution principle

Let q(x) be a property provable about objects of x of type T. Then q(y) should be provable for objects y of type S where S is a subtype of T.

objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.

I - Interface segregation principle

A client should never be forced to implement an interface that it doesn't use or clients shouldn't be forced to depend on methods they do not use.

D - Dependency Inversion Principle

one should "depend upon abstractions, [not] concretions."

A. High level modules should not depend upon low level modules. Both should depend upon abstractions.
B. Abstractions should not depend upon details. Details should depend upon abstractions.

Injection de dépendances

Pas bien !

    public class Client {
      private Service service;

      public Client() {
        this.service = new ServiceLieAUneDatabasePrecise();
      }

      public String greet() {
        return "Hello " + service.getName();
      }
    }
                    

    public class Client {
      private Service service;

      public Client(Service service) {
        this.service = service;
      }
        ...
    }
                    

Des frameworks permettent de faire "facilement" de l'injection de dépendances.

A voir si la complexité et les contraintes qu'ils apportent sont compensés.

Ne jamais coupler avec des librairies externes !

Jamais

Toujours utiliser des abstractions et des façades

Smells

Mieux

• SOLID principles
• null -> objet "vide"
• héritage -> compositions
• if/switch -> polymorphisme
• pas de singletons

Null

Ne retournez jamais null.

Soit le vide est une valeur possible et c'est un objet spécifique
("", un UserEmpty, emptylist ...)

Soit c'est une erreur et exception