3 ms·
There is a fine line between short code and readability, which is, by far, one of the most important factors of code quality. Say you have a code: $this->exe
by andreygrehov 10y ago
There is a fine line between short code and readability, which is, by far, one of the most important factors of code quality. Say you have a code:
$this->executeAfter(5);
This is a short code, but it's not readable. What the hell is 5 here? Let's improve it a little bit.
class Time {
protected $seconds;
private function __construct($seconds)
{
$this->seconds = $seconds;
}
public static function inSeconds($seconds)
{
return new self($seconds);
}
}
$this->executeAfter(Time::inSeconds(5));
I've just added ~10 lines of code, but the readability of executeAfter is way better. In addition, the executeAfter method is more flexible, since it can manipulate the Time object now.
This is a very basic example to illustrate what I meant when drawing a line between the short code and readability.
- segmondy 10y agoFor magic numbers, just use a well named variable, constant or configuration parameter. private const TIME_DELAY_SECONDS = 5; $this->executeAfter(TIME_DELAY_IN_SECONDS);
- gknoy 10y agoI'm a fan of named parameters, too: # python self.executeAfter(func=myCallback, delay_in_seconds=5) # or self.executeAfter(func=myCallback, after=delay(seconds=5))
- tdumitrescu 10y agoWhat this example is really lacking is a TimeFactoryFactoryAbstractSingleton to make it extra clear what that whole '5' thing is about.
- jamescostian 10y agoI believe you have missed the author's point - "Using more code is actually the lazy fix." The author would recommend something like: $this->executeAfter(5); // Execute after 5 seconds The above code is only 1 line, and is very clear. It does not add a brand new, unnecessary class, nor does it increase the surface area for bugs (adding a new class opens up the potential for new bugs, while adding a comment does not).
- andreygrehov 10y agoThis is not entirely true. It depends on the language, but the definition of executeAfter for the most of dynamic languages would be something like public function executeAfter($seconds) This code does not guarantee that the value of $seconds is correct, whereas this: public function executeAfter(Time $time) ...will help engineer to not make silly mistakes. It definitely depends on the language, but I personally like self-explanatory code and tend to think that inlined comments should only describe things that require explanation, such as complex logic.
- krapp 10y agoOr, you know, you could just rename it $this->executeAfterSeconds(5);
- elliotchance 10y agoActually, this is the kind of complexity and misunderstanding that makes software more difficult to maintain. If you step back and understand that the problem is the method name is not clear, creating more code to get around that is totally unnecessary - just fix the method name. The other repliers hit the nail on the head with better options. I personally would opt for the renaming of the method first: $this->executeAfterSeconds(5); If it was a third-party library or I was able to refactor the method itself I would go for constant named after what it represents, rather than what it is: $this->executeAfter(FIVE_SECONDS); As a last resort (like if you had some reason why would couldn't physically change the source) a comment would also be totally acceptable.