Useless loops

[Stormy]

So I am working through creating a calendar in JavaScript and I have a little nested loop that looks like this:

Code: Select all

// accumulate days by looping through the daysInMonths array
// and adding them as we go
for(i=1; i<=daysInMonths.length-1; i++){
   // make the secondary incrementer equal to the current daysInMonths index
   j = i;
   // count backwards from j and add each index
   // below current j in daysInMonths until j==0
   // assign the accumulated value to the current i index of cumDays
   while(j != 0){
      cumDays[i] = cumDays[i] + daysInMonths[j];
      j--
      }
}

But now that I have run that loop and it has calculated what it needed to, I find that I can just hardcode the values in the array rather than having them calculated by the loop. They are based on the days of each month so they don't change (except feb, which is accounted for anyway). It means that the loop will run like 90 times each time the calendar is opened, whereas I can hardcode the values and skip the loop entirely.

My question is this: Is it worth keeping that loop? It took me a while to figure it out and I think it's nice, but it is entirely unnecessary. Does 936 runs through a loop have any noticeable effect? What about on mobile devices? And is this relevant to the larger scale question of robust programming? Should I hardcode any values that only need to be calculated once ever? Or should programming have more tricky shit like loops and arrays to calculate the crap every time? How many loops does it take before you notice a difference in speed?

I know you compsci aficionados will know all about this...

[zombie@computer]

So I am working through creating a calendar in JavaScript and I have a little nested loop that looks like this:

Code: Select all

// accumulate days by looping through the daysInMonths array
// and adding them as we go
for(i=1; i<=daysInMonths.length-1; i++){
   // make the secondary incrementer equal to the current daysInMonths index
   j = i;
   // count backwards from j and add each index
   // below current j in daysInMonths until j==0
   // assign the accumulated value to the current i index of cumDays
   while(j != 0){
      cumDays[i] = cumDays[i] + daysInMonths[j];
      j--
      }
}

But now that I have run that loop and it has calculated what it needed to, I find that I can just hardcode the values in the array rather than having them calculated by the loop. They are based on the days of each month so they don't change (except feb, which is accounted for anyway). It means that the loop will run like 90 times each time the calendar is opened, whereas I can hardcode the values and skip the loop entirely.

My question is this: Is it worth keeping that loop? It took me a while to figure it out and I think it's nice, but it is entirely unnecessary. Does 936 runs through a loop have any noticeable effect? What about on mobile devices? And is this relevant to the larger scale question of robust programming? Should I hardcode any values that only need to be calculated once ever? Or should programming have more tricky shit like loops and arrays to calculate the crap every time? How many loops does it take before you notice a difference in speed?

I know you compsci aficionados will know all about this...

Nobody can tell you, that's the fun of programming. To find out, you have to test it. Reducing code is always good, but at what cost? Will the code file be inflated? I doubt anyone will notice code like this running a lot, but if you have lots of other code running... Each drop can add to the problem and may overflow the bucket... In reality a computer nowadays can run millions of lines like this per second, but the only way to be sure... is to test.

Anyway, if speed is a problem, mind the little things

Code: Select all

// accumulate days by looping through the daysInMonths array
// and adding them as we go
for(i=1; i<=daysInMonths.length-1; i++){


i<=daysInMonths.length-1
and
i<daysInMonths.length
do exactly the same but with 1 calculation per loop less.

cumDays[i] = cumDays[i] + daysInMonths[j];
and
cumDays[i] += daysInMonths[j];

again do exactly the same with one calculation less

Of course, the difference is minimal (may not even be worth the effort), but if you are strapped for speed in large loops, this is where you get the profit

[Stormy]

Thanks for the tips, I am trying to shorthand the hell out of everything because I get the feeling that downloading more lines of code will take longer than doing some extra calculations. For instance I just shortened this:

Code: Select all

if(i<=cumDays[1]){
// month is january
   calendar[i][2] = "Jan";
   // the third index (index 2) in the current day array holds the month name
   }
if(i>=cumDays[1] && i<=cumDays[2]){
// month is february
   calendar[i][2] = "Feb";
   // the third index (index 2) in the current day array holds the month name
   }
if(i>=cumDays[2] && i<=cumDays[3]){
   calendar[i][2] = "Mar";
   }
if(i>=cumDays[3] && i<=cumDays[4]){
   calendar[i][2] = "Apr";
   }
if(i>=cumDays[4] && i<=cumDays[5]){
   calendar[i][2] = "May";
   }
if(i>=cumDays[5] && i<=cumDays[6]){
   calendar[i][2] = "Jun";
   }
if(i>=cumDays[6] && i<=cumDays[7]){
   calendar[i][2] = "Jul";
   }
if(i>=cumDays[7] && i<=cumDays[8]){
   calendar[i][2] = "Aug";
   }
if(i>=cumDays[8] && i<=cumDays[9]){
   calendar[i][2] = "Sep";
   }
if(i>=cumDays[9] && i<=cumDays[10]){
   calendar[i][2] = "Oct";
   }
if(i>=cumDays[10] && i<=cumDays[11]){
   calendar[i][2] = "Nov";
   }
if(i>=cumDays[11] && i<=cumDays[12]){
   calendar[i][2] = "Dec";
   }

to this:

Code: Select all

// use incrementer j to loop through cumDays array
// and compare incrementer i to cumDays[j]
for(j = 0; j<=cumDays.length-1; j++){
// cumdays[1] = days passed in the year before january (31)
// so if i is less than 31, we are in January
// if i is less than 60 but more than 31, we are in february.
// and so it goes...
if(i>cumDays[j] && i<cumDays[j+1]){
   // the second index in the current day array holds the name of the month.
   // cumDays[] and namesOfMonths[] line up, so take the string in
   // namesOfMonths[] and assign it to the second index of the current day array.
   calendar[i][2] = namesOfMonths[j+1];
   // if i is spot on the amount of the current cumDays value, (cumDays[f])
   // then it will be undefined because the above comparison only looks at
   // greater or less than. So we assign border values to the current month
   }else if (i == cumDays[j]){
      calendar[i][2] = namesOfMonths[j];
      }

take out the comments for the minimized version and that's 6 lines of code, down from 20 or so. I'm pretty happy with that. Also that loop there told me that <= and < are not the same, but it might also be my logic that counteracted it.

[SM Sith Lord]

As far as compressing the code, I think human readable and maintainable is more important. There is probably some sort of "compiler" out there that can reduce your code to a fraction of the file size, like that is used for JQuery. You and the next guy plainly understanding each step of the code just by looking at it would make fixing or adding stuff in a lot easier in the future.

[SM Sith Lord]

Taking a 2nd look at your "compressed" code, it isn't really confusing. It's just the mammoth amount of comments that makes it look insane heh. Having that huge list of IF statement was messy, having the array of month names is much nicer. I just won't have file size in mind when trying to compress code.

[ScarT]

I would've written the code like this (same code, different comments and formatting).

Code: Select all

for(j = 0; j <= cumDays.length - 1; j++)
{
   // cumdays[1] = days passed in the year before january (31)
   // less than 31, we are in January - less than 60 but more than 31, we are in february.
   if( i > cumDays[j] && i < cumDays[j + 1] )
   {
      // the second index in the current1 day array holds the name of the month.
      // cumDays[] and namesOfMonths[] line up, so take the string in
      // namesOfMonths[] and assign it to the second index of the current day array.
      calendar[i][2] = namesOfMonths[j + 1];
   }
   // if i is spot on the amount of the current cumDays value, (cumDays[f])
   // then it will be undefined because the above comparison only looks at
   // greater or less than. So we assign border values to the current month
   else if (i == cumDays[j])
   {
      calendar[i][2] = namesOfMonths[j];
   }
}

Its just a lot easier to figure out.

[Stormy]

You're right, that is way easier to understand. I have been writing the comments as I write the code so they tend to be a bit fragmented. Also towards the end I started getting frustrated with bugs that are inherent in the system because my logarithm was flawed, and started to comment less or not at all. Incidentally, here is my finished calendar, it's all javascript so you can view the source in the linked .js file in the head:

http://www.modelsforthemasses.com/jsfid ... endar.html

It's not particularly re-usable as it only became an object towards the end of writing it so it's a bit hap hazard but as my first foray into object oriented programming, I'm pretty proud of it.