Comment by kazinator

Comment by kazinator a day ago

25 replies

View on Hacker News

> If there’s an if condition inside a function, consider if it could be moved to the caller instead

This idle conjecture is too rife with counterexamples.

- If the function is called from 37 places, should they all repeat the if statement?

- What if the function is getaddrinfo, or EnterCriticalSection; do we push an if out to the users of the API?

I think that we can only think about this transformation for internal functions which are called from at most two places, and only if the decision is out of their scope of concern.

Another idea is to make the function perform only the if statement, which calls two other helper functions.

If the caller needs to write a loop where the decision is to be hoisted out of the loop, the caller can use the lower-level "decoded-condition helpers". Callers which would only have a single if, not in or around a loop, can use the convenience function which hides the if. But we have to keep in mind that we are doing this for optimization. Optimization often conflicts with good program organization! Maybe it is not good design for the caller to know about the condition; we only opened it up so that we could hoist the condition outside of the caller's loop.

These dilemmas show up in OOP, where the "if" decision that is in the callee is the method dispatch: selecting which method is called.

Techniques to get method dispatch out of loops can also go against the grain of the design. There are some patterns for it.

E.g. wouldn't want to fill a canvas object with a raster image by looping over the image and calling canvas.putpixel(x, y, color). We'd have some method for blitting an image into a canvas (or a rectangular region thereof).

neoden 18 hours ago

> If the function is called from 37 places, should they all repeat the if statement?

the idea here is probably that in this case we might be able to split our function into two implementing true and false branches and then call them from 21 and 16 places respectively

Reply View 1 reply
  • kazinator 9 hours ago

    That's possible only if the condition is constant-foldable.

    You can achieve it by turning the if part into an inline function.

    Before:

      function(cond, arg)
  {
    if (cond) { true logic } else { false logic } 
  }
    after:

      inline function(cond, arg) { cond ? function_true(arg) : function_false(arg) }
    Now you don't do anything to those 37 places. The function is inlined, and the conditional disappears due to cond being constant.
    Reply View 0 replies
panstromek 18 hours ago

The keyword here is `consider`. The article targets a somewhat specific design problem where this comes up, especially when you use tagged unions or something similar.

Reply View 0 replies
PaulRobinson a day ago

If the function is called from 37 places, you need to refactor your code, but to answer your question on that point: it depends. DRY feels like the right answer, but I think we'd have to review an actual code example to decide.

On examples where you're talking about a library function, I think you have to accept that as a library you're in a special place: you're on an ownership boundary. Data is moving across domains. You're moving across bounded contexts, in DDD-speak. So, no, you look after your own stuff.

EnterCriticalSection suggests a code path where strong validation on entry - including if conditions - makes sense, and it should be thought of as a domain boundary.

But when you're writing an application and your regular application functions have if statements, you can safely push them out. And within a library or a critical code section you can move the `if` up into the edges of it safely, and not down in the dregs. Manage your domain, don't make demands of other people's and within that domain move your control flow to the edge. Seems a reasonable piece of advice.

However, as ever, idioms are only that, and need to be evaluated in the real world by people who know what they're doing and who can make sensible decisions about that context.

Reply View 20 replies
  • kenjackson a day ago

    Refactoring due to being called more than N times seems very function dependent. As the prior author noted, I’d expect to call a lock function in some programs a lot. Likewise, memcpy. In fact I’d argue that well factored functionality is often called at many different call sites.

    Reply View 0 replies
  • CJefferson 16 hours ago

    I can't imagine a large program where no function is useful enough to be called more than 37 times. Memory allocation? Printing? Adding a member to a list? Writing to a file?

    I'm guessing you mean something else, or do you feel useful functions can't be called many times in the same program?

    Reply View 0 replies
  • jovial_cavalier a day ago

    Pray tell, how many places is appropriate to call the same function? Is 5 too many? How about 6? When I hit 7, I have to refactor everything, right?

    Reply View 8 replies
    • cakealert 20 hours ago

      This only applies to a situation where you have a function that requires dynamic checks for preconditions. I would suggest that such a function (or how it's being used) is likely a blight already, but tolerable with very few call sites. In which case checking at the call site is the right move. And as you continue to abuse the function perhaps the code duplication will prompt you to reconsider what you are doing.

      Reply View 6 replies
      • jovial_cavalier 10 hours ago

        So if a function dereferences a pointer, it doesn't make sense to check that it's not null inside the function?

        Unless there's an actual performance implication, this is all purely a matter of taste. This is the kind of broadly true, narrowly false stuff that causes people to destroy codebases. "I can't write it this way, because I have to push ifs up and fors down!!!" It's a totally fake requirement, and it imposes a fake constraint on the design.

        Reply View 5 replies
    • tylersmith 20 hours ago

      You don't need an explicit rule, you just need to be smarter than than the average mid-curve tries-too-hard-to-feel-right hn poster and realize when you're repeating a calling convention too much.

      Reply View 0 replies
  • worik a day ago

    > If the function is called from 37 places, you need to refactor your code,

    Really?

    I do not have to think hard before I have a counter exampl: authentication

    I call authenticate() is some form from every API

    All 37 of them

    Reply View 8 replies
    • bognition a day ago

      If you are explicitly calling authenticate() for each api, you’re doing it “wrong”. At that point you want implied authentication not explicit authentication. Why not move it to some middleware that gets called in every api call?

      Reply View 6 replies
      • kazinator a day ago

        Because then you are calling middleware_caching_auth_broker() from 37 places instead of authenticate(). Just the name has changed, not the 37.

        Reply View 5 replies
    • kazinator a day ago

      The strongest interpretation of the remark is not that you need to refactor because you have a function called 37 times (which is likely a good thing) but rather that if you think you need to move an if statement into or out of it, you face refactoring.

      Reply View 0 replies