Comment by PaulRobinson

Comment by 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.

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 15 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 19 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 9 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
  
  kazinator 6 hours ago
  
  If there is a performance implication of moving the if into the callers or not, you can do it with an inline function.
  static inline int function(blob *ptr, int arg) { if (ptr == NULL) return ERR_NULL; return real_function(ptr, arg); }
  Just like that, we effectively moved the if statement into 37 callers, where the compiler may be smart enough to hoist it out of a for loop when it sees that the pointer is never changed in the loop body, or to eliminate it entirely when it sees that the pointer cannot be null.
  
  Reply View | 0 replies
  
  cakealert 8 hours ago
  
  IMO you should assert it's not null. There should never be a circumstance where you pass a null pointer to a function.
  
  Reply View | 3 replies
tylersmith 19 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
  
  bognition 36 minutes ago
  
  No that’s not how this works. You register the middleware with your web framework and it gets called as part of all web requests before the request hits your endpoints. This allows you to trust that authentication has been called for all api calls
  
  Reply View | 0 replies
  
  all2 a day ago
  
  But that's ok because the calls are hidden from the programmer.
  I'm not sure if my response is serious or tongue-in-cheek. Maybe a bit of both.
  
  Reply View | 0 replies
  
  KPGv2 21 hours ago
  
  > Because then you are calling middleware_caching_auth_broker() from 37 places
  No you aren't. You aren't really calling it from anywhere. The framework you're using, which you aren't writing, is calling the registered middleware.
  The topic here is complexity for the code structure because it's called from 37 different places. A registered middleware doesn't run into that issue because it doesn't get called anywhere that "code structure complexity" matters.
  Your reasoning is isomorphic to "I'm calling a bit shift millions of times because I have written some code in a programming language." Technically true but not what we're talking about here.
  
  Reply View | 2 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