Book Reviews Clean Code - herougo/SoftwareEngineerKnowledgeRepository GitHub Wiki

Overall thoughts

  • I only read/skimmed the first 133 pages.
  • I stopped reading because I wasn't learning anything. Most of the messages were simply common sense I picked up along the way (e.g. good code usually shouldn't need comments based on how you name functions/variables, etc).
  • There are some questionable recommendations though (e.g. splitting one function into 7-9 smaller functions).

Specific Notes / Comments (pages are by pdf)

  • page 56: Recommends to "Avoid words like Manager, Processor, Data, or Info in the name of a class."
    • Why? It follows this by saying, "A class should not be a verb." I still don't get why naming something with "Manager" in the name is bad?
  • Suppose you have a Shape class. You define a Circle and Square subclass. Now you implement a class called AreaCalculator which takes in a shape and computes the area (using a switch statement to determine the shape). What if you need a PerimeterCalculator too? This will also have a switch statement.
    • This is bad because??? It violates the open-closed principle? You have N switch statements?
    • The solution is to create a ShapeFactory class which has the only switch statement, and implement area and perimeter methods on each of the shapes.
    • quote from book: "My general rule for switch statements is that they can be tolerated if they appear only once, are used to create polymorphic objects, and are hidden behind an inheritance relationship so that the rest of the system can't see them."
  • page 58: if a parameter is a field of a class (e.g. Address), consider prefixing that parameter with that class to give context (e.g. addrFirstName parameter name).
  • page 65: Listing 3-3 is questionable (changing a 6-line function to a 4-line function).
  • page 76: output arguments should be avoided
    • my interpretation: functions should not modify their arguments. Methods should only modify their object (not the arguments).
  • page 76: command query separation
    • Functions should either do something or answer something, but not both.
    • Consider public boolean set(String attribute, String value);
    • This leads to odd statements like if (set("username", "unclebob"))...
      • What does this mean? 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”?
if (attributeExists("username")) {
  setAttribute("username", "unclebob");
  ...
}
  • page 77: use exceptions and avoid error codes
    • code with error codes
if (deletePage(page) == E_OK) {
  if (registry.deleteReference(page.name) == E_OK) {
    if (configKeys.deleteKey(page.name.makeKey()) == E_OK){
      logger.log("page deleted");
    } else {
      logger.log("configKey not deleted");
    }
  } else {
    logger.log("deleteReference from registry failed");
  }
} else {
  logger.log("delete failed");
  return E_ERROR;
}
  • code with exceptions
try {
  deletePage(page);
  registry.deleteReference(page.name);
  configKeys.deleteKey(page.name.makeKey());
}
catch (Exception e) {
  logger.log(e.getMessage());
}
  • page 103: Listing 4-8 PrimeGenerator.java
    • Took single function 40ish line function and split it into 8 functions. I see what he's going for (this definitely applies to much longer functions). But, in this example, I'd much rather read one complete function top to bottom. Furthermore, those 7 private functions are only used for that one public function, so we're bloating the function count and scattering the logic across the file.

Internet of Bugs Video on Criticizing Clean Code

source: https://www.youtube.com/watch?v=8ncQrGuunHY&t=515s

"The point of code maintainability is to make the code easier to work on for someone who is unfamiliar with it, not making it easier for you to write it in the first place."

It's not about some hypothetical change in the future, it's about how to find where in the code changes need to be made in a way that's isolated from other components.

Developers tend to learn code bases bottom up (find a specific place in the code, then go up levels to get a sense of what's happening) instead of top down (the way clean code seams to do it).

Write code so that changes in one "slice" does not affect other slices.

  • What does this mean???

Design code to avoid the bug whack-a-mole situation (whrn fixing one bug over here leads to a new different bug over there).

The value of maintainable code is not to write perfect code (that's basically impossible). It's to allow people to easily find bugs, and fix them in a way so that they don't come up again.

Examples of bad advice from the book

  • abstracting with too many functions
  • swallowing exceptions without the function caller knowing (for "abstraction simplification" purposes)