5
\$\begingroup\$

From an integer X representing a time duration in seconds produce a simplified string representation. For example, given X=100, you should output: "1m40s"

Use the following abbreviations w,d,h,m,s to represent : *1w is 1 week

  • 1d is 1 day
  • 1h is 1 hour
  • 1m is 1 minute
  • 1s is 1 second

Only the two largest non-zero units should be used. Round up the second unit if necessary so that the output only has two units even though this might mean the output represents slightly more time than X seconds. Write a function: function solution (X); that, given an integer X, returns a string representing the duration.

Examples:

  1. Given X=100, return "1m40s"
  2. Given X=7263, return "2h2m". (7263s=2h1m3s, but this uses too many units, so we round the second largest unit up to 2h2m)
  3. Given X=3605, return "1h5s"

Notes:

  • If a single unit is sufficient, then the output should only include one unit. ex. X=3600 outputs "1h", not "1h0s".

  • The output should be as close as possible to the real time without being less than X. Ex.: X=86461 is "1d1m1s" so the output would be "1d2m". Not "1d1h".

  • There should always be some output. The empty string() is not the outcome of any X.

    const countZeros = (array) =>
     array.reduce((acum, curr) => {
      if (curr === 0) {
        acum++;
      }
      return acum;
    }, 0);
    
    function timeConverter(x) {
     x = Number(x);
     var arrayT = [];
     var arrayInitials = ['w', 'd', 'h', 'm', 's'];
     var w = Math.floor(x / (3600 * 24 * 7));
     var d = Math.floor((x % (3600 * 24 * 7)) / (3600 * 24));
     var h = Math.floor(((x % (3600 * 24 * 7)) % (3600 * 24)) / 3600);
     var m = Math.floor((((x % (3600 * 24 * 7)) % (3600 * 24)) % 3600) / 60);
     var s = Math.floor((((x % (3600 * 24 * 7)) % (3600 * 24)) % 3600) % 60);
    
     arrayT.push(w);
     arrayT.push(d);
     arrayT.push(h);
     arrayT.push(m);
     arrayT.push(s);
    
     let i = arrayT.length - 1;
     while (countZeros(arrayT) <= 2) {
       if (arrayT[i] !== 0) {
         arrayT[i - 1]++;
         arrayT[i] = 0;
       }
       i--;
    }
    
    const time = arrayT.reduce((acc, curr, index) => {
      if (curr !== 0) {
        acc += curr + arrayInitials[index];
      }
      return acc;
    }, '');
    return time !== '' ? time : '0s';
    }
    
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I like the code in general. It's logical in its sequence of operations and the code formatting/style expresses that.

Functionality:

  • Most importantly, there's a bug in the code where you limit the number of units. Try the number: 43199. (I haven't tried to fix it.)

Code clarity:

  • If there's one thing I would avoid, it's using reduce() (I'm generally opposed to that method). To replace it you could try use Array .filter(fn), .length and .join(''), or write a for of loop.
  • If there is another thing, I would try to express the logic of limiting of the number of units more closely to the formulation of the assignment. But I suspect that will be solved when you address the bug.

UPDATE 1

How to fix the bug? I would suggest you to first try to look at it without loops or extra function. Try to write code for the following:

  1. Take the value corresponding to the largest time unit.
  2. Take the value corresponding to the second largest time unit. (unit-2)
  3. Check the value corresponding to the next time unit (unit-3): If it's more than 0, correct the value of 2. (rounding up)*
  4. Combine the results. Think of the case where there are less than 2 values.

Then you can see if you're code has obvious duplication, and try to wrap it in a loop or a function. Otherwise I'd keep it that way.

*Correct me if there's better wording: where I say "the value corresponding to a time unit", I mean the value of the portion of the input for a certain unit. Where I say "the value of a time unit" to describe the value inherent to that unit.


UPDATE 2

Alternatives to reduce

// 1. count the number of zero's
// a. original
// cons:
// - Not understood on first sight: Usage of the reduce function is not as expected.
//     Why? Reduction is a computer science concept: a series of values is reduced to a single value (e.g. count, sum, join).
//          This code does that for only a subset of the array.
//     I avoid reduce, but I use other array looping functions: `map`, `filter`, `some`,...
//     The added value of these methods, is that they express their intent in English.
//          You will benefit from them if you strictly use them as their name implies (see alternative 3.)
// - More advanced knowledge (I did not remember the order of parameters)
// - You do not have this code under full control.
//     This makes using a step-by-step debugger more difficult.
//     (In case you would have a more difficult calculation you can immagine a small update in requirements is more likely to have you to rewrite the whole loop.) 
var x = array.reduce((acum, curr) => {
    if (curr === 0) {
      acum++;
    }
    return acum;
  }, 0);
 
// b. alternative using loop
// so simple!
var x = 0
for (var it of array) {
    if (it === 0) {
        x++
    }
}

// c. alternative using array functions
// bonus: elegance :)
var x = array.filter(it => it === 0).length
// Do you see what I mean about these array looping functions?
//      Imagine you have a longer program and you notice a bug where you don't have get right subset of a list. You've probably found its source.

// 2. building the string
var units = ['w', 'd', 'h', 'm', 's']

// a. original
var formatted = array_duration.reduce((acc, curr, index) => {
    if (curr !== 0) {
      acc += curr + units[index];
    }
    return acc;
  }, '')

// b. alternative using loop
var formatted = ''
for (let i = 0; i < array_duration.length; ++i) {
    let value = array_duration[i]
    if (value !== 0) {
        formatted += value + units[i]
    }
}

// c. alternative using array functions
// I didn't find a good alternative using `map` and `join` unlike I suggested in comment.

UPDATE 3

Possible solution

function format(duration) {
    const units = [{
        symbol: 's',
        bound: 60,
    }, {
        symbol: 'm',
        bound: 60,
    }, {
        symbol: 'h',
        bound: 24,
    }, {
        symbol: 'd',
        bound: 7,
    }, {
        symbol: 'w',
        bound: undefined,
    }]

    var i_last = units.length - 1

    var duration_quotient = duration

    // array of objects to store calculation variables for a unit
    let a_unit_ctx = units.map(
        (unit, i) => {
            var is_last = i === i_last
            
            if (!is_last) {
                var divisor = unit.bound
                if (divisor == null) throw new TypeError()
        
                var value = duration_quotient % divisor
                duration_quotient = (duration_quotient - value) / divisor
            } else {
                value = duration_quotient
            }
         
            return {
                index: i,
                unit: unit,
                value: value,
            }
        })

    // find the index of the last value to render (needs to be corrected later)
    let a_nonzero = a_unit_ctx.filter(a => a.value > 0)
    let a_significant_provisional = a_nonzero.slice(-2) // keep the 2 largest units or less
    // are there more than 2 values => rounding needed
    let has_extra_nonzero = a_nonzero.length > 2
    let lowest_significant_provisional = a_significant_provisional[0]
    
    // round up
    if (has_extra_nonzero) {
        ++lowest_significant_provisional.value

        // rounding up 1 value up, can require multiple higher units to round up.
        // e.g. 23h59m59s => outputs 1d
        for (let i_current = lowest_significant_provisional.index; i_current < a_unit_ctx.length; ++i_current) {
            if (i_current === i_last) break

            let unit_ctx = a_unit_ctx[i_current]
            let is_overflow = unit_ctx.value === unit_ctx.unit.bound

            if (is_overflow) {
                unit_ctx.value = 0

                let higher_value_ctx = a_unit_ctx[i_current + 1]
                higher_value_ctx.value++
            }
        }
    }

    if (lowest_significant_provisional === undefined) {
        return '0s'
    } else {
        // recalculate numbers to display
        // same as above
        let a_significant = a_unit_ctx.slice(lowest_significant_provisional.index)
        let a_shown = a_significant
            .filter(x => x.value > 0)
            .slice(-2)

        return a_shown.reverse().map(x => x.value + x.unit.symbol).join('')
    }
}


let w = 604800
let d = 86400
let h = 3600
let m = 60
let s = 1

function test (input, expected_output) {
    var actual_output = format(input)
    console.log(input, expected_output, actual_output, expected_output == actual_output)
}

test(100, '1m40s')
test(7263, '2h2m')
test(3605, '1h5s')
test(43199, '12h')

var input = w - d + 5 * s
test(input, '6d5s')

var input = w - d + 2 * m + 5 * s
test(input, '6d3m')

var input = w - 3*d - 1*h + 2*m + 5*s
test(input, '4d')

var input = 28*d + 2*h + 1*m + 5*s
test(input, '4w3h')

var input = 2 * w - 5*s
test(input, '2w')

var input = 2 * w - 2* m - 5*s
test(input, '2w')

var input = 0
test(input, '0s')
```
\$\endgroup\$
9
  • \$\begingroup\$ Yes, you are right about the bug...now I don't know actually how to proceed...I've been staring at the monitor for about an hour now and nothing \$\endgroup\$ Commented Nov 13, 2022 at 12:00
  • 1
    \$\begingroup\$ Why the opposition to reduce? It's a pretty standard CS textbook primitive. I don't think I'd use it in this case, but it would be interesting to see why the blanket opposition. (E.g. python moved it out of the standard namespace 'because people just use it to implement sum() and other ways are clearer'. But numerical code uses it quite a lot.) \$\endgroup\$
    – 2e0byo
    Commented Nov 13, 2022 at 17:37
  • \$\begingroup\$ @LeandroTabak I updated with a possible approach to fix the bug. Success. \$\endgroup\$
    – pikachu
    Commented Nov 13, 2022 at 21:24
  • \$\begingroup\$ @2e0byo I tried to understand why they added it years ago. I never did. The (big) advantages of a simple var + forloop approach: the flow is from up to down, and the intermediate results never leave the code block you're working in. What advantages can reduce() give you? I think it's just a theoretical trick to get rid of variables (at the time there was a bit hype around functional programming). \$\endgroup\$
    – pikachu
    Commented Nov 17, 2022 at 9:07
  • \$\begingroup\$ @pikachu I think the answer is exactly that---for the functional style. Some people dream in functional, apparently. Personally I'm not that clever, although it's clearly the right tool sometimes. I wouldn't have used it here, because it reimplements a fancy sum (using if within reduce somehow smells) with a blank string as an intermediate value, and there are much clearer (IMHO) ways of writing that logic. But in some domains (numerical) /languages (lisps) reduce is pretty much standard. Each to their own I guess... \$\endgroup\$
    – 2e0byo
    Commented Nov 17, 2022 at 23:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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