Hacker News new | ask | show | jobs
by brendino 5068 days ago
While the implementation of this would be pretty straightforward, the method signature you're asking for seems convoluted, in my opinion. Here's why:

- "add1" is not very descriptive of what the method actually does, since it adds one, but only sometimes. This title makes it sound like the method simply increments each value by one.

- When you say "if it is zero, do this, but if it is positive, do this...", this thinking creates a "secret handshake" in your code, so a new developer would have to read the method in depth to understand what it does in each case.

- If this method will have widespread use in your code for arrays, consider extending the array class with the method, so that you can call it directly on an instance of an array.

I propose you change the method signature to:

  increment_where_value_equals(value, options = {})
So you could call it like:

  increment_where_value_equals 1, { :direction => :forward, :limit => 2 }
or even like:

  increment_where_value_equals 1
While the implementation may get a little more complicated this way, you're making the method signature much more clear and descriptive, which will lead to better code overall.