10 ms·

My experience is that I run into a lot of relatively junior programmers who are concerned about clean code. Is my code clean? How do I organize my code? How do

My experience is that I run into a lot of relatively junior programmers who are concerned about clean code. Is my code clean? How do I organize my code? How do I make it clean? Should we clean up this code? I almost never want to use the word “clean” when I’m talking about code. These days, when someone asks me to review code, and they start talking about “clean” code, I shift the discussion to two points—code should be correct and easy to understand. I think “correct and easy to understand” is a much more useful rubric than “clean”. Obviously it’s still subjective. Code that is easy to understand for you may be hard for me to understand. Likewise, code that is correct for your use cases may be incorrect for my use cases. But it’s much easier to come to an agreement about “correct and easy to understand”, or at least communicate issues with the code using that basis. Like, “you shouldn’t use boolean flags as parameters, you should use enums” becomes “I can‘t understand the meaning of true/false at the call site, so let’s use an enum instead”. This gives us a very clear, articulable basis for how we talk about code quality. Of course, “correct and easy to understand” is not the be-all and end-all for describing good code. It’s just a nice substitute for the horribly vague, terribly subjective “clean”.

cush 4y ago
It can definitely be confusing to hear "clean" when communicating in a professional setting. That said, in the book he covers correctness and understandability in nearly every paragraph. It's all the book is about really. It's called "clean" because it's concise and a catchy book title.
- dietrichepp 4y ago
  If you say the book “covers correctness and understandability in nearly every paragraph” then I’m convinced we must be talking about different books. For example, the book presents a rule for class names: > Classes and objects should have noun or noun phrase names like Customer, WikiPage, Account, and AddressParser. Avoid words like Manager, Processor, Data, or Info in the name of a class. A class name should not be a verb. What a proclamation! We could connect this to correctness and understandability. But the book does not. You could say that it is harder to understand a class named “SomethingManager” because “manager” is vague, and more specific words are usually preferred. Instead, the book presents a rule, without explanation or example. Kind of like the Strunk & White book on style, and if you like Strunk & White, then we disagree on at least two books. That’s my general complaint about the book. Too many proclamations, too much doctrine, not enough explanation or foundation.
  - cush 4y ago
    There's an entire chapter on naming things, with tons of examples. Why would a class be anything but a noun or noun phrase? It's not a hot take
    - dietrichepp 4y ago
      I think you may have missed the context for why I brought it up in the first place.
      - cush 4y ago
        I didn't. Ironically, you (or the juniors you're referring to in your original comment) missed the context in the book. You said he presents it as a rule, without explanation or example, but you happened to cherry pick the one paragraph from the 13 pages of Meaningful Names that didn't have an explanation or example. You're right, the problem is vagueness, and that context is already set a few pages prior. It would be redundant to repeat the same reasoning.
  - eurasiantiger 4y ago
    Let’s say my site has a download image feature that embeds per-user digital licenses into downloaded images at download time. This might take some processing so there is a queue and an abstraction representing the downloaded file. There are also several endpoints serving different types of image files. What shall I call the unit of code orchestrating these download processes, if not ImageDownloadManager?
    - cnorthwood 4y ago
      ImageDownloadPreparer?
      - eurasiantiger 4y ago
        ImageDownloadManager also communicates with the frontend about its state, so it’s not a completely preparatory process.
tommica 4y ago
Clarity! That's my goal for my code: https://m.youtube.com/watch?v=6sNmJtoKDCo https://m.youtube.com/watch?v=6sNmJtoKDCo
- Chris_Newton 4y ago
  IMHO, that was an excellent talk, well presented and insightful. I found your argument starting around 28:00 interesting. I’m not familiar with the specific tools you’re using and their idioms, so it’s possible that I’m missing some context here. As a general principle, I like to make it very clear where code is doing I/O and what is happening with any data involved. If this function receives data in one format, transforms that data into another format and finally sends that data to the database, then to me that seems like a useful separation of concerns and a clear representation of the overall behaviour. Maybe I wouldn’t choose the same name/structure as User.registration_changeset in your example, but the principle of hiding the implementation details of the data transformation seems helpful, for the same reason it’s helpful to see Repo.insert() instead of five lines of SQL at this level of the code.
  - tommica 4y ago
    Ah, the talker is not me, I'm just a fan of the concept
    - Chris_Newton 4y ago
      Ah, sorry, I misunderstood. In any case, it seems we probably agree! :-)
thaumasiotes 4y ago
> code should be correct and easy to understand. I think “correct and easy to understand” is a much more useful rubric than “clean”. Obviously it’s still subjective. Code that is easy to understand for you may be hard for me to understand. There's a deeper problem here that has driven a lot of disputes over the years. "Ease of understanding" is not a stable property of code. If you have two separate pieces of code that are easy to understand, and the only thing you do with them is combine them, your resulting one piece of code may also be easy to understand. Or it might be next to impossible to understand.
zepolen 4y ago
> Like, “you shouldn’t use boolean flags as parameters, you should use enums” becomes “I can‘t understand the meaning of true/false at the call site, so let’s use an enum instead”. That's not the reason to use enums instead of parameters for functions, rarely will you not be able to understand what the bool arg means in: create_user(..., is_admin: bool) If you don't then you need a better IDE. The reason to use enums and for that matter structs is that as requirements evolve, inevitably you will need to extend the code base and at some point `is_admin` alone won't cut it, you'll need an is_moderator etc. This leads to two things, one is function signature pollution and unnecessary extra validation code, eg. what happens if both is_admin and is_moderator are set? What about the functions relying on the old signature? Perhaps you make separate versions of the functions, eg. create_admin_user and create_moderator_user and keep the old one with the flag, then you can keep backward compatibility...but at the cost of an api that makes people go insane when using it. More likely you'd then go to an enum which leads backward incompatible changes, and likely a refactor, which in the event of a library can break dependent code and in all cases is a lot more work than just using an enum in the first place. Try to write code for the future reader and writer of the codebase. An enum will allow readers to immediately understand that there is a [normal, admin] distinction and adding a moderator would be as simple as adding a [normal, moderator, admin] in there. Then, assuming you've written your code in a proper manner, your IDE will tell you all the places where a moderator needs to be accounted for in the code base.
- habitue 4y ago
  At the call site, unless it's a keyword argument, you don't know what the parameter is named. IDEs help with this of course, but it's not just the "2 options becomes 3 options" situation that recommends against them
  - zepolen 4y ago
    That's not a problem with bool arguments, that's a problem with the language and tooling.
    - xupybd 4y ago
nathias 4y ago
> Obviously it’s still subjective. Code that is easy to understand for you may be hard for me to understand. there is nothing obvious about this, I think it's very objective what unreadable means (mostly it's complicated without a reason)
ebiester 4y ago
What is easy to understand code? What if you have three people in the same code base who write "easy to understand" code for them, but inconsistent with each other? Even if those three all understand each other while writing it, this will not continue as the team changes and people have to onboard. As such, it isn't sufficient to have a linting standard: great codebases should have a consistent mental model. "Clean" code, within the context of Bob Martin "Clean", is one consistent mental model. I don't like all of it, but it gives one coherent top-to-bottom model.
- SergeAx 4y ago
  Code is a kind of language. It is entirely possible to tell if person speaks clearly, easy to understand, or not. You may even train an ML model on Wikipedia section "Simple English". The same applies to code. It is not 100% deterministic metric, but it is relatively easy to argue and have a consensus upon.
  - sshine 4y ago
    > it is relatively easy to argue and have a consensus upon If you’re a reasonable person, yes. But when you’re not, all kinds of crazy patterns evolve. E.g. a boss saying “I’m perfectly ok with functions that take more than 100 arguments.” (Compiler does not even support this.) and code being copy-pasted dozens of times being perfectly readable, but is a sink-hole when you refactor. Clean Code (Martin’s book, and the movement in general) is my weapon when stubborn colleagues will not listen to reason. Sometimes they will listen to authority.
  - wadd1e 4y ago
    >It is entirely possible to tell if person speaks clearly, easy to understand, or not. Just thinking out loud here, but isn't correctness(grammar and spelling) the only thing we all(mostly) agree on? For example, someone well versed with old English literature may consider Shakespeare easy to understand, but I certainly wouldn't. I think "speaks clearly, easy to understand" are just as subjective as our notions of clean/idiomatic/readable code.
vaughan 4y ago
I think the problem with talking about understandable code is that we only talk about textual representation. We should break out of the ide and visualize code more. I think then we would see the tangled mess we are creating. And then if we started talking about code that was cleanly visualized, we would truly have understandable code. We need two-way sync for visualizations of all data structures and data flows. And then we need to see the actual data values inline as it flows through the system as we read the code. Wallaby.js is a great leader in this space. I found it too hard to setup and too slow - but I’m convinced this applied to the entire stack is the future. We draw diagrams on the whiteboard to explain every system but they are completely absent when we actually code.
- kcartlidge 4y ago
  > And then we need to see the actual data values inline as it flows through the system as we read the code ... Wallaby.js is a great leader in this space It's a very under-appreciated feature, but I know what you mean. Alongside NCrunch, Wallaby is one of my most valuable purchases. For those not familiar with the feature being referred to, as a continuous test runner one of the great benefits is that the IDE plugins show the values of anything (variables etc) in your code, live. I don't mean some kind of hover-for-intellisense, but at the end of each line you see all the values for all the variables all the time. So in real-time, as you code, any tests covering the code you're changing are being run and you can see holistically how everything you're working on is changing as you type. Basic test runners run continuously on the command line. Better ones run in the background of your IDE, possibly adding coverage markers etc. Wallaby goes one step further and actually decorates your code with real values live in the IDE as you work. It's very good (though, yes, tricky to set up and a little slow).
ploppyploppy 4y ago
*Testable, correct & easy to understand
treeman79 4y ago
Been around awhile. Reached the point of. I really don’t care what style bosses use. Just be consistent. Can’t stand being asked multiple times to change something depending on what order people review code. I’ve learned to love auto linting for the sole reason that it instantly shuts down any discussion on styling.

cush 4y ago

It can definitely be confusing to hear "clean" when communicating in a professional setting. That said, in the book he covers correctness and understandability in nearly every paragraph. It's all the book is about really. It's called "clean" because it's concise and a catchy book title.

dietrichepp 4y ago
If you say the book “covers correctness and understandability in nearly every paragraph” then I’m convinced we must be talking about different books. For example, the book presents a rule for class names: > Classes and objects should have noun or noun phrase names like Customer, WikiPage, Account, and AddressParser. Avoid words like Manager, Processor, Data, or Info in the name of a class. A class name should not be a verb. What a proclamation! We could connect this to correctness and understandability. But the book does not. You could say that it is harder to understand a class named “SomethingManager” because “manager” is vague, and more specific words are usually preferred. Instead, the book presents a rule, without explanation or example. Kind of like the Strunk & White book on style, and if you like Strunk & White, then we disagree on at least two books. That’s my general complaint about the book. Too many proclamations, too much doctrine, not enough explanation or foundation.
- cush 4y ago
  There's an entire chapter on naming things, with tons of examples. Why would a class be anything but a noun or noun phrase? It's not a hot take
  - dietrichepp 4y ago
    I think you may have missed the context for why I brought it up in the first place.
    - cush 4y ago
      I didn't. Ironically, you (or the juniors you're referring to in your original comment) missed the context in the book. You said he presents it as a rule, without explanation or example, but you happened to cherry pick the one paragraph from the 13 pages of Meaningful Names that didn't have an explanation or example. You're right, the problem is vagueness, and that context is already set a few pages prior. It would be redundant to repeat the same reasoning.
- eurasiantiger 4y ago
  Let’s say my site has a download image feature that embeds per-user digital licenses into downloaded images at download time. This might take some processing so there is a queue and an abstraction representing the downloaded file. There are also several endpoints serving different types of image files. What shall I call the unit of code orchestrating these download processes, if not ImageDownloadManager?
  - cnorthwood 4y ago
    ImageDownloadPreparer?
    - eurasiantiger 4y ago
      ImageDownloadManager also communicates with the frontend about its state, so it’s not a completely preparatory process.

tommica 4y ago

Clarity! That's my goal for my code: https://m.youtube.com/watch?v=6sNmJtoKDCo https://m.youtube.com/watch?v=6sNmJtoKDCo

Chris_Newton 4y ago
IMHO, that was an excellent talk, well presented and insightful. I found your argument starting around 28:00 interesting. I’m not familiar with the specific tools you’re using and their idioms, so it’s possible that I’m missing some context here. As a general principle, I like to make it very clear where code is doing I/O and what is happening with any data involved. If this function receives data in one format, transforms that data into another format and finally sends that data to the database, then to me that seems like a useful separation of concerns and a clear representation of the overall behaviour. Maybe I wouldn’t choose the same name/structure as User.registration_changeset in your example, but the principle of hiding the implementation details of the data transformation seems helpful, for the same reason it’s helpful to see Repo.insert() instead of five lines of SQL at this level of the code.
- tommica 4y ago
  Ah, the talker is not me, I'm just a fan of the concept
  - Chris_Newton 4y ago
    Ah, sorry, I misunderstood. In any case, it seems we probably agree! :-)

thaumasiotes 4y ago

> code should be correct and easy to understand. I think “correct and easy to understand” is a much more useful rubric than “clean”. Obviously it’s still subjective. Code that is easy to understand for you may be hard for me to understand. There's a deeper problem here that has driven a lot of disputes over the years. "Ease of understanding" is not a stable property of code. If you have two separate pieces of code that are easy to understand, and the only thing you do with them is combine them, your resulting one piece of code may also be easy to understand. Or it might be next to impossible to understand.

zepolen 4y ago

> Like, “you shouldn’t use boolean flags as parameters, you should use enums” becomes “I can‘t understand the meaning of true/false at the call site, so let’s use an enum instead”. That's not the reason to use enums instead of parameters for functions, rarely will you not be able to understand what the bool arg means in: create_user(..., is_admin: bool) If you don't then you need a better IDE. The reason to use enums and for that matter structs is that as requirements evolve, inevitably you will need to extend the code base and at some point `is_admin` alone won't cut it, you'll need an is_moderator etc. This leads to two things, one is function signature pollution and unnecessary extra validation code, eg. what happens if both is_admin and is_moderator are set? What about the functions relying on the old signature? Perhaps you make separate versions of the functions, eg. create_admin_user and create_moderator_user and keep the old one with the flag, then you can keep backward compatibility...but at the cost of an api that makes people go insane when using it. More likely you'd then go to an enum which leads backward incompatible changes, and likely a refactor, which in the event of a library can break dependent code and in all cases is a lot more work than just using an enum in the first place. Try to write code for the future reader and writer of the codebase. An enum will allow readers to immediately understand that there is a [normal, admin] distinction and adding a moderator would be as simple as adding a [normal, moderator, admin] in there. Then, assuming you've written your code in a proper manner, your IDE will tell you all the places where a moderator needs to be accounted for in the code base.

habitue 4y ago
At the call site, unless it's a keyword argument, you don't know what the parameter is named. IDEs help with this of course, but it's not just the "2 options becomes 3 options" situation that recommends against them
- zepolen 4y ago
  That's not a problem with bool arguments, that's a problem with the language and tooling.
  - xupybd 4y ago

nathias 4y ago

> Obviously it’s still subjective. Code that is easy to understand for you may be hard for me to understand. there is nothing obvious about this, I think it's very objective what unreadable means (mostly it's complicated without a reason)

ebiester 4y ago

What is easy to understand code? What if you have three people in the same code base who write "easy to understand" code for them, but inconsistent with each other? Even if those three all understand each other while writing it, this will not continue as the team changes and people have to onboard. As such, it isn't sufficient to have a linting standard: great codebases should have a consistent mental model. "Clean" code, within the context of Bob Martin "Clean", is one consistent mental model. I don't like all of it, but it gives one coherent top-to-bottom model.

SergeAx 4y ago
Code is a kind of language. It is entirely possible to tell if person speaks clearly, easy to understand, or not. You may even train an ML model on Wikipedia section "Simple English". The same applies to code. It is not 100% deterministic metric, but it is relatively easy to argue and have a consensus upon.
- sshine 4y ago
  > it is relatively easy to argue and have a consensus upon If you’re a reasonable person, yes. But when you’re not, all kinds of crazy patterns evolve. E.g. a boss saying “I’m perfectly ok with functions that take more than 100 arguments.” (Compiler does not even support this.) and code being copy-pasted dozens of times being perfectly readable, but is a sink-hole when you refactor. Clean Code (Martin’s book, and the movement in general) is my weapon when stubborn colleagues will not listen to reason. Sometimes they will listen to authority.
- wadd1e 4y ago
  >It is entirely possible to tell if person speaks clearly, easy to understand, or not. Just thinking out loud here, but isn't correctness(grammar and spelling) the only thing we all(mostly) agree on? For example, someone well versed with old English literature may consider Shakespeare easy to understand, but I certainly wouldn't. I think "speaks clearly, easy to understand" are just as subjective as our notions of clean/idiomatic/readable code.

vaughan 4y ago

I think the problem with talking about understandable code is that we only talk about textual representation. We should break out of the ide and visualize code more. I think then we would see the tangled mess we are creating. And then if we started talking about code that was cleanly visualized, we would truly have understandable code. We need two-way sync for visualizations of all data structures and data flows. And then we need to see the actual data values inline as it flows through the system as we read the code. Wallaby.js is a great leader in this space. I found it too hard to setup and too slow - but I’m convinced this applied to the entire stack is the future. We draw diagrams on the whiteboard to explain every system but they are completely absent when we actually code.

kcartlidge 4y ago
> And then we need to see the actual data values inline as it flows through the system as we read the code ... Wallaby.js is a great leader in this space It's a very under-appreciated feature, but I know what you mean. Alongside NCrunch, Wallaby is one of my most valuable purchases. For those not familiar with the feature being referred to, as a continuous test runner one of the great benefits is that the IDE plugins show the values of anything (variables etc) in your code, live. I don't mean some kind of hover-for-intellisense, but at the end of each line you see all the values for all the variables all the time. So in real-time, as you code, any tests covering the code you're changing are being run and you can see holistically how everything you're working on is changing as you type. Basic test runners run continuously on the command line. Better ones run in the background of your IDE, possibly adding coverage markers etc. Wallaby goes one step further and actually decorates your code with real values live in the IDE as you work. It's very good (though, yes, tricky to set up and a little slow).

ploppyploppy 4y ago

*Testable, correct & easy to understand

treeman79 4y ago

Been around awhile. Reached the point of. I really don’t care what style bosses use. Just be consistent. Can’t stand being asked multiple times to change something depending on what order people review code. I’ve learned to love auto linting for the sole reason that it instantly shuts down any discussion on styling.