1
\$\begingroup\$

EDIT: Alternate answer on overflow: https://stackoverflow.com/questions/38487692/how-can-i-interpret-the-time-with-less-code/38492144#38492144

But I really want to keep Jonah's answer here too. Just let me know if I really have to close either this one or stackoverflow's question.


I really don't like sticking many ifs together. I think it looks like a child stacking. But if anyone knows how to do the same thing I'm doing with this code but with less lines, it would be great.

var time = data[i].data4;
                    var summary = '<td>' + time + ' minutes since last login.</td></tr>';
                    if(time >= 60) {
                        var hour= tiempo / 60;
                        summary = '<td>' + Math.floor(hora) + ' hours since last login.</td></tr>';
                        if(hour>= 24) {
                            var day = hour/ 24;
                            summary= '<td>' + Math.floor(day) + ' days since last login.</td></tr>';
                            if(dia >= 7) {
                                var week= day / 7;
                                summary = '<td>' + Math.floor(week) + ' weeks since last login.</td></tr>';
                                if(week >= 4) {
                                    var month = week / 4;
                                    summary = '<td>' + Math.floor(mes) + ' months since last login.</td></tr>';
                                }
                            }
                        }
                        s += summary;
                    } else {
                        s += summary;
                    }

For some context: data[i].data4 is from a column that is an operation of DATEDIFF(mi, someDate, getdate()).

As you can see, the returned results would be in minutes and from then on I start checking how many hours, days, and so on from that and I would even love to add between each line an (if it's greater than 1) so I can change the wording to singular instead of plural but I think that's too many lines just for a decision of whether to simply write 'minute' instead of 'minutes'.

\$\endgroup\$
  • 2
    \$\begingroup\$ I would have loved to review your code but the code is incomplete- Could you add some more information on how the jquery is called and also how data gets initialised. \$\endgroup\$ – Siobhan Jul 20 '16 at 22:31
  • \$\begingroup\$ I highly suggest you give moment.js a try. It's pretty powerful and offers a wide variety of options. \$\endgroup\$ – Alex L Jul 21 '16 at 0:44
1
\$\begingroup\$

Here's a rewrite without any ifs:

function humanTime(minutes) {
  var units          = ['months' , 'weeks' , 'days' , 'hours'];
  var minutesPerUnit = [40320    , 10080   , 1440   , 60];
  var timeInUnits    = minutesPerUnit.map(x => Math.floor(minutes / x));
  var time           = timeInUnits.find(x => x > 0);
  var unit           = units[timeInUnits.indexOf(time)];
  return time + ' ' + unit;
}

Note I removed the <td>s from this code as it's mainly a view (formatting) concern. This allows this function to remain more focused: it simply takes a number of minutes and converts it to hours/days/weeks/months as appropriate.

You might also consider returning an object of the form: {time: 6, unit: 'days'} instead of a string as I have, to make the separation of data and view even stronger.

\$\endgroup\$
  • \$\begingroup\$ This is actually very close to what I'm looking for, no if's nested infinitely into one another. I do get where you are going with this code and I take that x is meant to be initialized on minutesPerUnit right? But I keep getting syntax error if I paste it just like this and 'x is not initialized' when I change => to >=. So I think I'm not too sure about what you mean to express within the .map(). Could you just rewrite those parts for me? \$\endgroup\$ – dreami Jul 21 '16 at 14:14
  • \$\begingroup\$ oh you must be using a browser that doesn't support es6 (safari, maybe?). if you try this in chrome or firefox it will work. otherwise you have to transpile it with babel, or just rewrite the fat arrow functions to use normal js functions. \$\endgroup\$ – Jonah Jul 22 '16 at 0:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.