search

bitwalker / timex_ecto

An adapter for using Timex DateTimes with Ecto
MIT License
162 stars 68 forks source link

Question: Should :error be replaced with {:error, reason} on failed calls? #61

Closed taylonr closed 7 years ago

taylonr commented 7 years ago

When I was looking at Timex.Ecto.Date.load for my last PR, I noticed if I called it without a parameter that it considered valid, it just returned :error I wasn't sure if I passed in the wrong value, or there was something wrong with the conversion or what.

What are your thoughts on returning a tuple of {:error, reason}?

My biggest concern would be that if anyone is matching off of just :error then their code would now break.

For reference, Timex.to_date("2017-02-01") returns {:error, :invalid_date}

bitwalker commented 7 years ago

I always prefer verbose errors like you requested, however the reason for the use of :error is because that is what the type specification for custom Ecto types declares it should return in the case of error conditions. If that has changed though, I'm definitely open to making the errors better.

taylonr commented 7 years ago

Makes sense. I didn't think to look at Ecto spec.