Skip to content
This repository has been archived by the owner on Oct 7, 2021. It is now read-only.

Should clamping be the default behavior for a new Time object? #18

Closed
oldfartdeveloper opened this issue Jan 24, 2017 · 7 comments
Closed

Comments

@oldfartdeveloper
Copy link
Contributor

Referencing the README:


Constructing Dates

Use date to construct Date values. If given invalid values for
the month and day, they are both clamped and the nearest valid date is
returned.


This is wrong on the basis of the #create function is doing two things:

  1. Creating an instance of the data.
  2. Modifying it arbitrarily if any of the function's parameters are wrong.

This would be tolerable if there was any other way to create the object, but unfortunately there isn't. Hence, in my mind the #create function is broken, and if I want to use this module, I have to write a wrapper around this class. I will probably instead fork the module, modify it, and then publish it as a "better" competitor. As the maintainer, I say #ugh

Here's my suggested solution that still allows clamping:

  1. The default behavior needs to be that wrong inputs throw a Result. Now the api is providing me exactly what I need to know. It can tell me what value(s) are wrong and I can program a correction any way I want "the Elm way".
  2. If I want the admitted simplicity of "clamping", then the solution is to add a function, #create_clamped, that will do exactly that, AND the name of the function explains what it is doing.

Everybody wins.

@oldfartdeveloper oldfartdeveloper changed the title Should clamping be the default behavior for a new Time object Should clamping be the default behavior for a new Time object? Jan 24, 2017
@sporto
Copy link

sporto commented Jan 24, 2017

@oldfartdeveloper what you are proposing sounds like a good solution, +1 for that

@ericgj
Copy link

ericgj commented Jan 24, 2017

I disagree with this change. I don't want to have to deal with Results for basic date arithmetic and construction, it would make it very clunky to use. The failure cases are such outliers I can't really imagine them happening in real situations. Have you ever had a real situation trying to construct a date with arbitrary numbers? Clamping seems like a fine solution for cases that don't practically exist and thus avoiding a burdensome Result to unpack.

@Bogdanp
Copy link
Collaborator

Bogdanp commented Jan 24, 2017

@ericgj is right. In fact, initially I had used Maybes for all constructor functions. This made the API extremely annoying to use for virtually no benefit.

@Bogdanp
Copy link
Collaborator

Bogdanp commented Jan 24, 2017

Also worth pointing out is that if you do need validation you can always use the provided isValid{Date,Time} functions.

@oldfartdeveloper
Copy link
Contributor Author

oldfartdeveloper commented Jan 24, 2017

@ericgj @Bogdamp For those who want simplicity, I offered the new API function, create_clamped which could be included in this library. Why does that not give you what you want while keeping the basic API more robust for those that must handle errors differently?

@ericgj
Copy link

ericgj commented Jan 24, 2017

If you insist on providing Result-returning functions for no clear benefit, I would at least make them the exceptional ones, and the plain value returning functions the default.

Why don't you try it out first and see if you like dealing with all date operations within a Result? The original developer has said he has done as much and backed it out. On the basis of actual use.

@oldfartdeveloper
Copy link
Contributor Author

After thinking about it and talking it over with some other Elm developers, I came to the following realizations:

  • The date function, as is, is suitable when you have confidence that the entries will be correct. Such would certainly be the case if the inputs were obtained from a reliable date picker, for example.
  • As @ericgj and @Bogdanp has vociferously asserted, handling a Maybe or Result is frequently a pain when you don't think you need to worry about it.
  • Date errors can only be generated in two places:
    • Human input within the Elm code. This is why that should be done by a date picker which precludes any human error.
    • API's to systems outside the Elm code: server, 3rd-party, etc. The proper way to handle those is validation of the JSON.
  • In both cases, error processing at the date creation is not convenient.

Hence, I'm not going to attempt to rename date or its present API definition (though I admit that the clamping feature gives me the chills, I can't think of a good enough justification to object that much).

Closing this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants