1. 2
  1.  

  2. 11

    I don’t fully agree that smaller functions are always the best idea. (I don’t really agree with a lot of the advice from Clean Code but that’s another matter.) It’s a piece of advice that I think works really well when you’re operating with pure or mostly side effect-free functions, but falls apart when you introduce side effects.

    It is often really hard to express exactly what a function that performs multiple side effects is doing with a succint and meaningful name that other people can easily understand, so often you end up with tons of poorly named functions that you have to dig through and read to actually understand what a piece of code is doing when having a longer function would’ve made it all more clear and explicit.

    Functions are effectively a way of naming a piece of code, as the OP exemplifies, so IMO the advice should be: name things that can be given a good, meaningful, and obvious name, either with variables or with functions, but if you can’t find a meaningful name maybe consider just writing a longer function or repeating some code.

    1. 3

      I appreciate your critique.

      so IMO the advice should be: name things that can be given a good, meaningful, and obvious name, either with variables or with functions, but if you can’t find a meaningful name maybe consider just writing a longer function or repeating some code.

      I like the sentiment here. It’s very pragmatic, and usually real world use cases aren’t as neat and tidy as examples created for a blog post.

    2. 11
      private static bool IsMissing14Days(int year)
        {
          return year == 1918;
        }
      

      This 100% needs a comment. Something like

      /* Year Soviets switched from Julian to Gregorian.
       * They went from 1918-01-31 → 1918-02-14
       * See XYZ for more info on the change
       * See ABC for why this matters to us
       */
      private static bool IsMissing14Days(int year)
        {
          return year == 1918;
        }
      
      1. 1

        Thanks for that feedback and example. Definitely food for thought. At first I was going to categorize this example as “Explanation of Intent” and I think comments like that are important. But really, the comment explains context that is true outside of the business rules of the application. I agree with you that such comments are very useful, and I didn’t talk about those kinds of comments in my blog post, which is a regrettable oversight.

        1. 2

          The last line in the example comment is the one that I often see omitted: When should I be calling this function and why?

          I’d also be inclined to restructure the code to a function like adjustForJulianToGregorianMigration() or similar because there’s only one reason that you’d ever want to call isMissing14Days and that is so that you can apply the adjustment. If you do make the test and the action separate things then there are three possible bugs:

          • Failing to do the check and doing the adjustment unconditionally
          • Doing the check correctly but then doing the adjustment incorrectly.
          • Forgetting to do the check and the adjustment.

          If you combine the two then the last of these is the only possible bug.

      2. 10

        Code may change and the corresponding comment often remains as it was. Except now the comment is misleading. This makes maintaining the code unnecessarily difficult.

        If you’re not adjusting comments in your code when you’re changing the code, then you aren’t maintaining the code.

        1. 5

          The same kind of programmer who doesn’t update comments also will not rename the function when he changes its behaviour.

          1. 1

            That’s a fair point. If the comment is there, then the programmer should keep it accurate or remove it if it’s not necessary.

          2. 2

            And, it accounts for when the calendar changed from Julian to Gregorian.

            The calendar changed from Julian to Gregorian at different times in different places. See https://en.wikipedia.org/wiki/Old_Style_and_New_Style_dates