Add floor and ceil functions to Float

[bitwalker]

Let me know if you spot anything wrong with this, I think the tests are fairly comprehensive, but if you can think of any edge cases I may have missed, I'll certainly add tests for them.

Right now this will take either an integer or a float, truncate it as part of the floor/ceil operation, then convert it back to a float. The result of calling Float.floor or Float.ceil will always be a float this way. I think that makes more sense than returning an integer, but let me know if that's an incorrect assumption.

[josevalim]

Thanks @bitwalker! I have a couple things: we don't need to return error. Just use when is_float or is_integer in the function guard clause and it should be enough. Also, I believe this should return an integer and be placed in the Integer module. What do you think?

[josevalim]

Ah, if you receive an integer, it should be a no-op. :) That should help simplify stuff. Finally, we should probably add a test case for "-0.32".

[meh]

@josevalim I think the proper place is Float, it's an operation on a float even though it returns an integer.

[josevalim]

@meh That's a fair point, thanks. Let's keep them in float, but with no :error and make it a no-op for integers.

[bitwalker]

Sure thing, I'll push those changes shortly!

[bitwalker]

Let me know if that looks good. I'm not sure how I feel about not returning :error for invalid input, as is done elsewhere in the Float module (see parse), but for now it just returns the input value, untouched.

[meh]

@bitwalker in the case of parse it makes sense to return an error in case of failed parse, since you could be giving it unchecked inputs.

In the case of floor and ceil you know what type you're passing to it.

[bitwalker]

Good point, I suppose I'm a bit overzealous with validation sometimes, and you're right it probably doesn't make sense for this. My only thought on returning an error from floor or ceil is that consumers probably don't want an obtuse error message from :erlang.trunc if they do something stupid like pass a string to one of these functions, returning an error tuple like {:error, :non_float_input} or something might make it a little more obvious what the problem is.

[meh]

@bitwalker that's why you only add two heads with is_integer and is_float guards, so it fails early when you pass something else.

[josevalim]

You probably want this:

def ceil(num) when is_integer(num), do: num
def ceil(num) when is_float(num) do
  ...
end

This way, if anything else besides float/integer is given, it will raise like :erlang.round does.

[bitwalker]

Oh man, I can't believe I forgot using function guards, I'll fix that up and add that test :)

[bitwalker]

If you guys think this looks good I can squash the commits prior to merging.

[josevalim]

It looks great, please squash and I will merge! :D

[bitwalker]

Squashed!

[bitwalker]

Thanks!