Fix invalid date issue without catch or raise exception #289
Conversation
|
👍 |
3 similar comments
|
👍 |
|
👍 |
|
👍 |
|
smooth 👍 |
|
nice 👍 |
|
👍 |
1 similar comment
|
👍 |
| @@ -24,10 +24,12 @@ def call(params) | |||
|
|
|||
| private | |||
| def params_to_date(year, month, day, hour, minute) | |||
| return nil if [year, month, day].any?(&:blank?) | |||
| if [year, month, day].any?(&:blank?) || !Date.valid_date?(*[year, month, day].map(&:to_i)) | |||
There was a problem hiding this comment.
You are generating the same array twice, can you refactor and reuse the same?
There was a problem hiding this comment.
I think about it, I just try to solve the problem with minimum code, but I will do it, above have the same array too.
There was a problem hiding this comment.
Blank strings become 0 when coerced via to_i. Given that any 0 is understood as an invalid date, what do you think about skipping the blank? check? Maybe I'm missing something?
|
👎 |
| return nil if [year, month, day].any?(&:blank?) | ||
| if [year, month, day].any?(&:blank?) || !Date.valid_date?(*[year, month, day].map(&:to_i)) | ||
| return nil | ||
| end | ||
|
|
||
| if hour.blank? && minute.blank? |
There was a problem hiding this comment.
if you are handling dates, why are you concerned about hour and minute?
|
Haha, great but since when do we have this 👍 on my gems?! 😆 |
|
Cool, so this catches invalid dates without an exception? That's great! Any concerns? |
|
Yeah, it will. And no concerns, it's ruby 1.8 compatible. It will return |
|
I just put the tests from #267. |
|
@halan But here you test invalid dates: https://github.com/apotonick/reform/pull/289/files#diff-ad5370b7e7280e5aeb58a9412a4ade3bR20 and I can't see any Please join https://gitter.im/trailblazer/chat for a quick chat! 🍻 |
|
👏 |
This solve #267 without catch generic exceptions or any other. I use the same test at the other PR.