mavoweb / mavo

Create web applications entirely by writing HTML and CSS!
https://mavo.io
MIT License
2.83k stars 178 forks source link

Negative values in duration() should produce a more meaningful result than "0 ms" #925

Open LeaVerou opened 1 year ago

LeaVerou commented 1 year ago

Options:

Note that we cannot do things like "2 hours ago" because that wouldn't work with localization.

And while I'm at it, "0 ms" is almost never desirable for duration(0). We should return something else, though not sure what.

DmitrySharabin commented 1 year ago

Note that we cannot do things like "2 hours ago" because that wouldn't work with localization.

Actually, this way of handling negative values makes the most sense from my perspective. That was my first thought when I saw the issue title, to be honest. 😀 Would it be possible to use this platform API to handle the localization issue you mentioned?

It seems to me, that can also cover the duration(0) case.

LeaVerou commented 1 year ago

Oh that would be really cool. We can do that when the argument is negative and keep the current behavior when it's positive. That does make it a little more intensive to implement though. Are you interested in implementing it @DmitrySharabin? Or @barishnamazov since you were the one that pointed this out, perhaps implementing that would be of interest?

BarishNamazov commented 1 year ago

@DmitrySharabin's idea is really great, but I was wondering if this breaks the consistency of duration's usage. For positive values, duration returns an amount of time, but if we return 2 hours ago for negative values, the result would be a point in time. Is it okay that the units don't match? If the code is like I slept ${duration(end_time - start_time)} or Sleep duration: ${duration(end_time - start_time)}, positive values definitely makes sense, but negative values would be very confusing (first example is especially interesting, changing the meaning of the sentence). I would personally prefer seeing a negative amount of time (e.g. -2 hours). I think what would work the best for duration(0) would be 0 unit where unit is provided (if not, I think we can use hours).

Let me know what you think!

DmitrySharabin commented 1 year ago

Yes, I also see that inconsistency. So I agree.

What if we follow option 2 (with the negative value as the result) and provide a relative_time() (or whatever we will call it) function for those who need it? It will cover a bunch of other use cases with relative time (not only in the past, but in the future as well).

What do you think?

joyously commented 1 year ago

I think the user shouldn't have to be careful which way they subtract two times to get the duration, so the function should always return a positive amount of time, not a point in time. If you want a separate function like Dmitry says, then the order would matter, and the result would be x ago, x from now, or now.

BarishNamazov commented 1 year ago

I like the idea of having another function like relative_time for describing a point in time, but I am not sure how I feel about duration returning a positive time to a given negative value. If the function was specified as duration(time1, time2, unit), then I would agree that it should just return the absolute amount of time between time1 and time2. Given the current usage, though (i.e. user doing the subtraction themselves), I am leaning more towards to returning a negative value (this is, if I take off my get-things-done coder hat and put on my spec-caring software designer hat).

However, even with the get-things-done coder hat, while I agree that duration returning a positive value because of a wrong order of subtraction is useful and fast, I'm worried it might lead to subtle bugs. Let's consider this case: I want to calculate the duration between start (=1 PM) and end (= 4 PM), but also want to add 1 hour. Writing duration(start - end + 1 * hours()) would return 2 hours, while the answer is actually 4 hours. Returning -2 hours to the user would reveal it easily that something is wrong, but the user might not even notice the issue otherwise.

My two cents.

joyously commented 1 year ago

Although I don't comprehend a negative duration, I can see the point you are making about subtle math errors in the parameters passed. However, I was thinking more along the lines of the target user being a non-programmer, and the use case of a list of user entered dates that you are charting (like a task time sheet or a competition). The easy way is to pass end - start, but if it's user entered data, you don't know which one is which, and a duration is actually always positive, so the function should reflect that.

BarishNamazov commented 1 year ago

One thing about JavaScript I don't like is that we cannot pass named optional parameters to functions, like in Python, i.e. I wish we could do duration(start - end, absolute=true). My target is also non-programmers, but assuming they are building a web application, I would guess they have used some kind of computer language before (e.g. minimally HTML/CSS), and can understand these issues happening. A non-programmer debugging also would be quite hard, so I am still leaning towards the safe side. This is options and my evaluations:

My proposal: we separate cases by making a function for each of them. So, duration will be either case 1 or 2, and a new function with relevant name will be the other case. Personally, I would prefer having duration as case 2 and having absolute_duration as case 1, but as long as we provide support for both, I wouldn't mind much. It's a matter of wording and naming.

LeaVerou commented 1 year ago

One thing about JavaScript I don't like is that we cannot pass named optional parameters to functions, like in Python, i.e. I wish we could do duration(start - end, absolute=true).

You can! In JS you use object literals, like duration(start - end, {absolute: true}). In Mavo, you can do duration(start - end, absolute: true).

  • if negative duration is needed, the user needs to write a conditional (e.g. I want to make a table where I see how fast I finished the race compared to others: so positive and negative matters, where positive and negative means I was faster or slower by result or the other way, depending on the subtraction)

Oh that's a good point.

  • Case 2. duration returns negative/positive amounts: forces users to provide the correct order, but easier to notice the mistakes.

    • user makes a mistake and most times notices the bug, and hopefully by seeing a negative value understanding the issue, but still putting effort
    • user spends more mental effort to come up with the right formula
    • if the data is not labeled (i.e. we don't have what is start and what is end), the user needs a conditional statement

My proposal: we separate cases by making a function for each of them. So, duration will be either case 1 or 2, and a new function with relevant name will be the other case. Personally, I would prefer having duration as case 2 and having absolute_duration as case 1, but as long as we provide support for both, I wouldn't mind much. It's a matter of wording and naming.

I think I'm convinced we should return a negative duration in those cases, but I'm not convinced we need a separate absolute_duration() function. People can always do duration(abs(value)) if that's what they want.

LeaVerou commented 1 year ago

@BarishNamazov do you want to try and submit a PR for this? It's a really easy one, so it would be a good learning opportunity.

BarishNamazov commented 1 year ago

Sounds great, will do soon!

BarishNamazov commented 1 year ago

While implementing the fix for negative duration, I realized that duration also can return multiple units if terms is a number, which is hard to represent with negative values. What do we return? -1 months, -2 hours seems a little weird (and just -1 months, 2 hours is technically wrong). Any ideas?

BarishNamazov commented 1 year ago

While I was doing that, I noticed possible enhancements for duration:

Let me know if you think an issue should be opened for any of these, I can open and work on it!

DmitrySharabin commented 1 year ago

While implementing the fix for negative duration, I realized that duration also can return multiple units if terms is a number, which is hard to represent with negative values. What do we return? -1 months, -2 hours seems a little weird (and just -1 months, 2 hours is technically wrong). Any ideas?

What if we wrap the absolute value in parentheses and add the minus sign before it? Like so, -(1 month, 2 hours).

LeaVerou commented 1 year ago

While implementing the fix for negative duration, I realized that duration also can return multiple units if terms is a number, which is hard to represent with negative values. What do we return? -1 months, -2 hours seems a little weird (and just -1 months, 2 hours is technically wrong). Any ideas?

I think the only reasonable option is for everything to be negative. As you point out, only the first being negative is actually incorrect. Negative durations are weird in general, so yeah, of course multiple ones are weirder. The idea is that this will show authors they've done something wrong (usually subtracted dates in the wrong order), not that this will be user facing.

While implementing the fix for negative duration, I realized that duration also can return multiple units if terms is a number, which is hard to represent with negative values. What do we return? -1 months, -2 hours seems a little weird (and just -1 months, 2 hours is technically wrong). Any ideas?

What if we wrap the absolute value in parentheses and add the minus sign before it? Like so, -(1 month, 2 hours).

The value returned when terms > 2 is actually an array, so that wouldn't work (at least not without a custom toString(), which IMO is would be overengineering it).

LeaVerou commented 1 year ago

While I was doing that, I noticed possible enhancements for duration:

  • I think it is a good idea to have unitless optional parameter that will return the value without units. Note that this will only apply when we are given a single unit.

The reason we don't is that there are other functions that return unitless durations (e.g. days()) without the complication of having a parameter that only works when another parameter has a certain value (which is a pattern that typically produces confusing APIs). Also, as a rule of thumb, it's usually preferable to design APIs around positive things, otherwise you end up with double negatives (unitless: false) which are generally hard to understand, in both natural languages and programming languages.

  • To support multiple units easier (in any given format), we can get them sorted from largest to smallest, and then call duration with unitless: true, and subtract the result value from timeLeft.

Not sure what you are proposing here, a code example of such a duration() call would help.

But while we're at it, after using duration() extensively myself, the one use case that keeps coming up again and again is that I don't want to specify the units explicitly, but I want to be able to constrain them. E.g. some way to say "I want a duration in days, weeks, months", i.e. use automatic unit(s) but with constraints. Perhaps something like minUnit and maxUnit parameters. I should make an issue about that.

LeaVerou commented 1 year ago

Closed via #930