Javascript while loop infinite loop

Question

I'm not all that experienced of a code and I'm stuck on a the while loop towards the bottom of the block of code below

My code is supposed to get the date, check if today is a day when we don't ship (saturdays, sundays, holidays) and, if it is, add 1 day until until it finds the next day that we are on open on and write it to the document.

var target = new Date();
var targetDay = target.getDay();
var targetDate = target.getDate();
var targetMonth = target.getMonth();

function checkIfClosedOnTarget(targetDay,targetDate,targetMonth){
    var areOpenOnTarget = true;
    if(
         targetDay == 0 || 
         targetDay == 6  ||
         (targetDate == 1 && targetMonth == 0) || // New Year's Day
         (targetMonth == 4 && targetDate >= 25 &&  targetDay == 1) || // Memorial Day
         (targetMonth == 6 && targetDate == 4) || //Independence Day
         (targetMonth == 8 && targetDate <= 7 && targetDay == 1)|| //Labor Day
         (targetMonth == 10 && targetDate <= 28 && targetDate >= 22 && targetDay == 4)|| // Thanksgiving Day
         (targetMonth == 11 && targetDate == 25)
     ){
         areOpenOnTarget = false;
     }

    if(areOpenOnTarget){
        return true;
    }else{
       return false;
    }
};

function addDaysUntilNextOpenDay() {
     while(checkIfClosedOnTarget(targetDay,targetDate,targetMonth) == false){
     target.setDate(target.getDate() + 1);
     }
 };
addDaysUntilNextOpenDay();

document.write("<p>Next shipment will ship out on " + target.getMonth() + " " +   target.getDate + ", " + target.getYear) + " at 4:00pm Pacific Standard Time ";

you should only return the logical sentence in the ''if'' statement of your ''checkIfClosedOnTarget'' function. The rest of code is redundant. — Arnaldo Ignacio Gaspar Véjar
– Arnaldo Ignacio Gaspar Véjar, Commented Apr 9, 2013 at 21:07
I think you could do a lot to clean up your code. For example, you have a method called checkIfClosedOnTarget which would imply that it returns true if closed, but it returns true if open. And it's wordy. Try renaming it to isOpen or isOpenOn. Then you really just need to return false in your big if statement, and true outside of it. There is no need for the areOpenOnTarget variable at all. — Mark Peters
– Mark Peters, Commented Apr 9, 2013 at 21:10

Amy · Accepted Answer · 2013-04-09 21:10:11Z

3

The problem is with this line target.setDate(target.getDate() + 1); you update the target but you never update the targetDay, targetDate, targetMonth variables... so the checkIfClosedOnTarget() function keeps getting passed the same values, resulting in the infinite loop.

So you might want to update them after you set the next day:

while(checkIfClosedOnTarget(targetDay,targetDate,targetMonth) === false){
     target.setDate(target.getDate() + 1);

     // update parameters
     targetDay = target.getDay();
     targetDate = target.getDate();
     targetMonth = target.getMonth();
}

answered Apr 9, 2013 at 21:10

Amy

7,5262 gold badges22 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Mark Peters Over a year ago

+1, you've identified the problem. However, I think it would be much, much cleaner to define targetDay et al as local variables in checkIfClosedOnTarget and just pass in target to that function.

Amy Over a year ago

@MarkPeters Agreed, there's different ways of passing the information in. Up to the OP to decide on how they want to best approach it.

Collectives™ on Stack Overflow

Javascript while loop infinite loop

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related