Is it clearer to sleep near a function call in a loop or in the function call itself?

Question

Is it clearer to sleep near a function call in a loop or in the function call itself? Personally I lean toward sleeping near the call rather than in the call, because there is nothing about "getApple()" that implies that it should sleep for some amount of time before returning the apple. I think it would be clearer to have:

for ( int i = 0; i < 10; ++i ) {
    getApple();
    sleep()
}

than...

for ( int i = 0; i < 10; ++i ) {
    getApple();
}

Apple getApple() {
    sleep(1);
    return new Apple();
}

Of course, this would be different if the method were getAppleSlowly() or something.

Please let me know what you think.

Some additional information (also in comments below, see comments):

The wait is not required to get an apple. The wait is to avoid a rate limit on queries per minute to an API, but it is not necessary to sleep if you're only getting one apple. The sleep-in-get way makes it impossible to get an apple without sleeping, even if it is unnecessary. However, it has the benefit of making sure that no matter what, methods can call it without worrying about going over the rate limit. But that seems like an argument for renaming to getAppleSlowly().

This is a subjective question, so it's not really a good fit for SO... — Oliver Charlesworth, Dec 28 '11 at 20:18
Yes, as Oli says. Does anthing else need apples without any wait, or is the wait absolutely required in order to get an apple? — Martin James, Dec 28 '11 at 20:19
The wait is not required to get an apple. The wait is to avoid a rate limit on queries per minute to an API, but it is not necessary to sleep if you're only getting one apple. The sleep-in-get way makes it impossible to get an apple without sleeping, even if it is unnecessary. However, it has the benefit of making sure that no matter what, methods can call it without worrying about going over the rate limit. But that seems like an argument for renaming to getAppleSlowly(). — Anonymous, Dec 28 '11 at 20:27
Don't sleep at all. Track what time the user last got an apple. If it's too soon, don't let the user get a new apple, otherwise go ahead and get the apple and update the time. If you sleep, you block everything that app could be doing or could want to do for no real benefit. — Jason Coco, Dec 28 '11 at 20:51

score 4 · Answer 1 · answered Dec 28 '11 at 20:17

4

The ideal is for one method to do one thing. Since getting apples and sleeping are two different things, I agree with you that it'd be better to have them be two different methods.

answered Dec 28 '11 at 20:17

yshavit

42,327
7
87
124

2

This is what I am thinking as well, I think you need a whole other class that manages the "is it too soon to send another query" think and somehow make it available down in getApple(). Put this on an AppleMaker interface and have an implementation that Throttles for production stuff and a implementation that doesn't for testing and the like. Voila...best solution. – Bob Kuhar Dec 28 '11 at 21:09

score 2 · Accepted Answer · answered Dec 28 '11 at 20:18

2

I would not put the sleep into the function itself as long as the function name does not suggest that there will be a delay.

There might be other callers to the function in the future who do not expect it to sleep.

answered Dec 28 '11 at 20:18

tobiasbayer

10,269
4
46
64

score 1 · Answer 3 · answered Dec 28 '11 at 20:33

I think this is a fair question, and it's very bad to put a sleep inside of a method where you might not know it's there (imagine trying to debug the slowness of your application in a few months when you have forgotten what you have done. The sleep should be only where you understand why it's sleeping (and presumably you have a good reason for that).

score 1 · Answer 4 · answered Dec 28 '11 at 21:04

This is a very interesting question and I think that both of your solutions are somewhat flawed. The "getApple(); sleep();" solution suffers from forcing each getApple() to pause before processing even if we will never again do a getApple(). The "sleep(); return new Apple();" solution has a similar overhead on the first Apple we get. The optimal solution is something like.

for ( int i = 0; i < 10; ++i ) {
    getApple();
}

Apple getApple() {
    long sleepTime = needToSleep();
    if ( sleepTime > 0 ) {
      sleep(sleepTime);
    }
    return new Apple();
}

/**
 * Checks if last query was made less than THRESHOLD ago and 
 * returns the difference in millis that we need to sleep.
 */
long needToSleep() {
    return ( lastQueryInMillis + THRESHOLD ) - System.currentTimeInMillis();
}

I would be inclined to get the whole "how long do I sleep to avoid the API throttle" thing behind some interface and make some other class wholly responsible for enforcing it.

Is it clearer to sleep near a function call in a loop or in the function call itself?

4 Answers4

Linked