Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am building a time tracking application and my requirement says that any hours past 40 total should be counted as overtime and not regular hours.

Which function is more clear? What can be done to clarify the function? Even the second one at ~ 10 lines seems wordy

Option #1

function calculateOvertime(days) {
    var totalHours = 0;
    var over40 = false;
    return $.map(days, function(hours) {
        var ot = 0;
        totalHours += hours;
        if (totalHours > 40 && !over40) {
            ot = totalHours - 40;
            hours = hours - ot;
            over40 = true;
        } else if (over40) {
            ot = hours;
            hours = 0; 
        }
        return { reg: hours, ot: ot};
    })    
}
calculateOvertime([16,16,16,16]);

Option #2

function calculateOvertime(days) {
    var hoursUntilOT = 40;
    return $.map(days, function(hours) {
        var ot = 0;
        hoursUntilOT -= hours;
        if (hoursUntilOT < 0) {
            ot = hoursUntilOT * -1;
            hours = hours - ot;
            hoursUntilOT = 0;
        }
        return { reg: hours, ot: ot};
    })    
}
calculateOvertime([16,16,16,16]);
share|improve this question
    
Does it need to be on a day-by-day basis? If overtime is defined here as > 40 hrs on a weekly basis, why not just sum the week's hours, and get "40 regular, 24 overtime" total. This code assumes the first two days are just "regular workdays", the 3rd is an 8 hour day plus overtime, and the 4th is all-overtime. In reality there'd be overtime each day, wouldn't there? Alternatively, define overtime as > 8 hrs per day. It just seems random to do per-day calculation when the overtime is defined per-week. –  Flambino Dec 10 '12 at 1:11
    
Thanks for the reply. Yes, it needs to be on a day-by-day basis as its displayed for each day of the week –  user148835 Dec 10 '12 at 2:25
    
...so again, why not define overtime as anything over 8 hours in a day? Again, if your workday is 9am to 5pm, but you work from 9am to 9pm, you've worked 8 regular hours, and 4 overtime. Yet your code could say that you've worked 12 regular hours with no overtime (wrong), or that everything is overtime (also wrong). –  Flambino Dec 10 '12 at 13:18
add comment

1 Answer 1

You can accomplish your task using only core Javascript, with no reference to JQuery code, in about that many lines. JQuery is overkill for this task.

I applied the following changes to make your code more readable:

  1. Reduced knowledge required to understand it (removed JQuery).
  2. Reduced the complex portions (see iCurrentOvertime).
  3. Added Hungarian notation; i.e. prepending integers with "i", arrays with "a", etc.
  4. Encased added values together within parentheses (e.g. iCurrentHours+iTotalHours becomes (iCurrentHours+iTotalHours)).

The new code is below:

function calculateOvertime(days) {
  var aHours = [];
  var iTotalHours = 0;
  for(var i=0;i<days.length;i++){
    var iCurrentHours = days[i];
    var iNewHours = (iCurrentHours+iTotalHours);
    var iCurrentOvertime = Math.max(0,(iNewHours - Math.max(iTotalHours,40)));
    iTotalHours = iNewHours;
    aHours.push({
      reg: (iCurrentHours - iCurrentOvertime),
      ot: iCurrentOvertime
    });
  }
  return aHours;
}
calculateOvertime([16,16,16,16]);
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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