1. 9
  1.  

  2. 6

    They sometimes are, and sometimes aren’t. It really strongly depends on the logic of the application in question. I don’t think this is a very well formulated argument, it’s just saying that you should have smaller functions. It could be true, but sometimes you cannot disentangle the logic of a function like that. For example if you have a dry_run flag, you’re saying run the function but do not actually make the request (or do whatever the function does). It would only be possible to make it into two functions if you duplicate the logic, something that would not be DRY at all.

    1. 6

      The author says it’s a code smell, not a code travesty. And in the example they provided, the code change was certainly a readability win. I often think about how I can clean up functions that have flag-guarded blocks interleaved together. Those get hairy fast.

      1. 1

        Yea, it definitely can be, and I’ve definitely written functions which do this badly and had to clean them up before–or had to clean up someone else’s before. But I think the article is making a point that’s not necessarily universalizable. In many cases, as a comment on the article says, you’d just be moving the conditional somewhere else.

        I think the general Ruby obsession with functions that are very small is a bit silly. While I do prefer smaller functions, it’s not always a reasonable choice and can easily create unnecessary complication and indirection in applications.

      2. 2

        It would only be possible to make it into two functions if you duplicate the logic, something that would not be DRY at all.

        Disagree. What’s the simplest way to achieve a dry run capability? I’d say split the code into:

        someAction(bool dryRun) {
          var state = prepare();
          if (!dryRun) {
            run(state);
          } else {
             dryrun(state);
          }
        }
        

        Don’t you find the following simpler:

        someAction() {
          var state = prepare();
          run(state);
        }
        
        someDryRun() {
          var state = prepare();
          dryRun(state);
        }
        

        Often the dry-run part is often as simple as return state, and the someDryRun method actually is the prepare method simplifying even further.

        1. 2

          You’re omitting the important part where dry run and run are basically the same, but ok

          1. 2

            You can use Free Monads to express this and then one would run it in different contexts.

            Another thing I often do is for anything with side effects toss that under an interface so the flow is always the same but it offloads side-effectful work to a value it gets as input. Then you can have the dry-run version of the interface and the real-version of the interface.

            1. 2

              So, how do I do this in C++ or Java or JS?

              EDIT: Talking about the free monads thing, not the interface thing.

              1. 1

                You can get pretty close with something like https://en.wikipedia.org/wiki/Interpreter_pattern

              2. 1

                That actually sounds kinda neat, I suppose that you could do something simple in C# like

                private void visitDeletableItems(List<Item> items, Action<Item> action)
                {
                  foreach (var item in items) // matching criteria or whatever
                  {
                    action(item)
                  }
                }
                public void deleteDryRun(List<item> items) { visitDeletableItems(items, item => {}); }
                public void delete(List<Item> items) { visitDeletableItems(items, item => { delete(item); }); }
                private void delete(Item item) { ... }
                
        2. 3

          See “boolean blindness”: https://existentialtype.wordpress.com/2011/03/15/boolean-blindness/

          The Qt API design principles, which are a great document, also discuss this: https://wiki.qt.io/API_Design_Principles#The_Boolean_Parameter_Trap

          1. 4

            In the Qt principles discussion it says they have moved from using bare boolean positional arguments to enum-based arguments. I agree that the former is hard to read, because it is not self-describing.

            The original post, however, seems to be talking about named arguments which, though using a boolean type are self-describing. That is, you see not just true, but preview: true which is much clearer.

          2. 3

            So the author’s proposed solution is to make every piece of code (that uses this function) include an if ... else ... then? Instead of a single branch? That seems like choice that only adds confusion everywhere instead of containing it to a singular location.

            1. 2

              I agree for the most part. However, having long method names have similar smelliness. I think that create_task is very readable. The fact that keyword arguments are used helps reduce confusion in the relevant parts of code. I.e., create_task("Something", send_email: false) is clear and concise. Thecreate_task` method could be an entry point where based on the flags, the other later mentioned (convert to private) methods are called to keep the logic clean and readable.