Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

Muster Mark

macrumors newbie
Original poster
Sep 9, 2008
8
0
Natick, MA.
I am relatively new to programing (I just got a book on C, and am trying to teach myself for fun), and I wrote a relatively complicated program (for me) that took in a date (in numerical form) and spat out the day of the week (the code is attached). When I was testing the code for bugs some really weird things started happening. It appeared that a variable was mysteriously changing its value from Month[12]=31 to Month[12]=10. I know these sort of things can happen from careless programming, so I tested what was going on. I inserted a

printf("%d %d %d",Month[12],month,Month[month]);

and I discovered a logical impossibility. The output from that printf() statement was this: 31 12 10. Now, this is absolutely impossible. If month=12 and Month[12]=31, then Month[month] HAS to equal 31.
I thought maybe it was a problem with xcode's 'simulator', for lack of knowing what it is actually called. So I tried to open the 'Unix Executable File' but it didn't exist. Where in most XCode project folders exists the executable file, was a file that looked like an executable file, until I clicked on it. Upon which it would turn into a C source code file, that when opened, looked like bunch of gibberish (lots of upside down question marks and such symbols, interspersed with bits of the original code for my program). After restarting (which obviously didn't help) I got so frustrated I just threw away the entire 'Developer' folder from the applications folder.

I had saved my code as a text file and I copied it to my brothers laptop. There, I started a new Xcode project on his computer and LO! The program ran beautifully without a hitch. Good, I thought, so my code was all right. I then took the executable file created on my bro's computer and copied it onto mine. When I ran this executable file (which ran fine on my bro's comp) it exhibited the same symptoms that my original test revealed to be a logical impossibility! In other words, it seems to run ok, but you get whacky feedback. What the heck is going on? I haven't re-installed Xcode yet, but I don't expect different results, because the executable file that ran fine on my brothers computer exhibited strange mistakes on mine.

Some probably relevant information: I have a :apple: mini with a 1.5ghz intel coresolo, that only had 2 GB free out of 60 (before deleting the Developer file). And my brother has a powebook G4 with a 1ghz PPC processor with ~10GB free out of 60. Both computers are running latest 10.5.

Thanks in advance for any answers.
 

Attachments

  • Calendar script.doc
    35.5 KB · Views: 198
Strange things happen when you overflow an array. Month only has 12 elements, and you're going off the end of it by subscripting it with 12. C has 0 based arrays. PPC is big-endian, Intel is little-endian. There are plenty of other differences between the platforms, etc. When you break something, it will behave unexpectedly.

I can try to figure out exactly why the "logical impossibility" occurred, exactly, but again, you were doing something with undefined behavior. The 2-bytes of memory immediately following Month should stay the same, but if it's being used by a library who knows. Also, a short should be printed with %hd, not %d, as %d prints a 4-byte integer, not a 2-byte integer. This could account for the issue as well.

-Lee
 
Thanks so much!! I think you hit the nail on the head there. I guess it was bound to be some stupid mistake like that. The thing is I already knew that the arrays start at zero. I have no idea why I didn't catch that. Oh well, now at least I am no longer frustrated :)

Cheers!
Strange things happen when you overflow an array. Month only has 12 elements, and you're going off the end of it by subscripting it with 12. C has 0 based arrays. PPC is big-endian, Intel is little-endian. There are plenty of other differences between the platforms, etc. When you break something, it will behave unexpectedly.

I can try to figure out exactly why the "logical impossibility" occurred, exactly, but again, you were doing something with undefined behavior. The 2-bytes of memory immediately following Month should stay the same, but if it's being used by a library who knows. Also, a short should be printed with %hd, not %d, as %d prints a 4-byte integer, not a 2-byte integer. This could account for the issue as well.

-Lee
 
You said you're just starting out, and this was a bit ambitious, but I wanted to point a few things out to try to help out.

Never use goto. There's no reason to ever use goto. We are enlightened. We can use control structures like if-then-else, or subroutines to free us of the spaghetti-code-inducing, readability-destroying, mind-bending, undebuggable goto construct.

Also, the logic in Days is a bit off. You are trying to find out how many days away from a base date the entered date is. It might be easier to have a series of subroutines that calculate the days difference from the given date in the given year vs. the base year, then from that month and year to the given month, etc. Better yet (as I do in the example i'll post), calculate the julian date, and just compare that the the julian base date. It's a bit more math, but less fiddling with the various components of the date.

Another thing to watch is you have 5 things called month.
MONTH, to be replaced with 9
Month, an int function taking a single parameter
month, an int parameter to Days
Month, an array of shorts containing the days in each month in the Days function
Month: a label for gotos (eeevil)

This leads to wholly unreadable bits of code such as:
Code:
Month :
  if (MONTH==month) goto Year;
  while (MONTH>month) //past
    {days-=Month[month]; month++;}

Read in english:
At label month, if month is equivalent to month, goto label year. While month is greater than month, subtract the value at index month of array month and increment month.

Ouch. You have plenty of room for longer variable names. Use it. Name things descriptively. And you never need labels, ever, so that takes care of one use right there.

I also have stylistic gripes, which I won't mention individually. The basic problem (other than gotos) is placement of turnstiles/curly braces ({ and }). I prefer them to be on the same line as the applicable construct (function name, if, while, etc.) with the closing on its own line. These might have been some other more minor things, but that really reduced the readability of the code.

The last thing I'll mention is something I've never seen before. You invoke main in an error condition to start execution again. There are different ways of expressing failure (generally a function return) and altering program flow. Calling main seems to be a really rough way of doing this, and difficult to follow. This can also lead to a very deep stack, as far as I can tell. As long as the user makes an error with the entry of the number of days, it seems like the call stack will get arbitrarily deep until you run out of room, and your program dies. I could be mistaken on this, calling main may smash the stack back down to the beginning, but one way or the other this is ill-advised.

I didn't fine tune the input, etc. but this is the "refactoring" I came up with, reformatting the code and switching to the use of julian date comparisons. I also removed switch (which is the same as a large series of if-then-else constructs) with a constant. You were really just using the passed in parameter as an index into an array anyway, but using a switch to do it. I just changed that to be more explicit. This was just to reduce the length of the code, really, which is not needed. It was just clearer to me to do it like this.

Code:
#include <stdio.h> 
#include <ctype.h> 
//JD= 367*2008 - ((7(2008 + (1)))/4) + 275*9/9 + 6 + 1721013.5
//JD= 736936 - 3515 + 275 + 6 + 1721013.5
//JD= 2454715.5
#define baseJD 2454716 //We will round up in all cases, assuming noon

int Days(int,int,int,int *); 
int printDayOfWeek(int); 
int printMonth(int); 
int makeJulian(int,int,int);

int main (int argc, char *argv[]) { 
  int d, m, y, ds;  
  int result=0;
  do {
    char ch='M'; 
    while (ch=='M'){ 
      printf("Enter the month (numerically).\n"); 
      scanf("%d", &m); 
      printf("Enter the day.\n"); 
      scanf("%d", &d); 
      printf("Enter the year.\n"); 
      scanf("%d", &y); 
      if(printMonth(m)) continue; 
      printf(" %d, %d is the date you want evaluated? Y=yes, N=no.\n",d,y); 
      while((toupper(ch)!='N')&&(toupper(ch)!='Y')) ch=getchar(); 
      ch=toupper(ch)-1; 
    } 
    result=Days(d,m,y,&ds);
  } while(result == -1);
  printf("%d/%d/%d ", m,d,y); 
  if(ds<0) { 
    printDayOfWeek(7-(-ds)%7); 
  } else { 
    printDayOfWeek(ds%7); 
  } 
  return 0;  
} 

int makeJulian(int year, int month, int day) { //Simplified by assuming date > 2/28/1900
  return 367*year - ((7*((month+9)/12))/4) + ((275*month)/9) + day + 1721014; //Assume noon, for ease
}

int Days (int day, int month, int year,int *numdays) {  
  short daysPerMonth[] = {0,31,(year%4==0 && year%100!=0)||year%400==0?29:28,31,30,31,30,31,31,30,31,30,31};
  *numdays=0; 

  if(day>daysPerMonth[month] || day < 1) { 
    printf("Invalid day\n\n"); 
    return -1;
  } 
  *numdays=makeJulian(year,month,day)-baseJD;
  return 0;
} 

int printDayOfWeek (int _d){ 
  char *nameOfDays[7] = {"Saturday","Sunday","Monday","Tuesday","Wednesday","Thursday","Friday"};
  if(_d < 0 || _d > 6) return 1;
  printf("%s\n",nameOfDays[_d]);
  return 0;
} 

int printMonth (int mo){ 
  char *nameOfMonths[12] = {"January","February","March","April","May","June","July","August","September","October","November","December"};
  if(mo < 1 || mo > 12) return 1;
  printf("%s",nameOfMonths[mo-1]);
  return 0; 
}

This is obviously a pretty large detour from your algorithm, but this was clearer for me to express in code than counting manually.

Note that this was meant as good-spirited. You said you were starting out, and there's no way to learn good form without seeing it and practicing it. I won't feign that I have the best style, etc. but hopefully this example has reasonable format, style, and logic.

-Lee
 
Thank you so so much

for taking the time to give me an in depth critique of my code. I really appreciate it. I know it takes a lot of time (especially with my bad form!), so thanks again, and don't worry about coming across as negative or something, such critique is exactly what I need (like I said, I'm learning from a book, and don't know anyone I can really talk to about it).

Regarding my logic in Days, I'm pretty sure that it is sound, albeit rather complicated. I've tested many dates against a calendar, and so far, they all have come out correct.

I will certainly try to clean up the code stylistically, and get rid of the repetitive naming of variables, and get rid of the invocation of main(), (should I do this by having it return 1 within that 'if' statement?). I didn't know that invoking main() was a no-no, so thanks for pointing it out. Oh, and your example of a calendar function is really helpful, thanks so munch for that!

When I get those kinks out of it, would you mind looking it over (I'll post it in this thread), and giving further pointers? Your advice is really helpful.

Thanks again.
 
for taking the time to give me an in depth critique of my code. I really appreciate it. I know it takes a lot of time (especially with my bad form!), so thanks again, and don't worry about coming across as negative or something, such critique is exactly what I need (like I said, I'm learning from a book, and don't know anyone I can really talk to about it).

Regarding my logic in Days, I'm pretty sure that it is sound, albeit rather complicated. I've tested many dates against a calendar, and so far, they all have come out correct.

I will certainly try to clean up the code stylistically, and get rid of the repetitive naming of variables, and get rid of the invocation of main(), (should I do this by having it return 1 within that 'if' statement?). I didn't know that invoking main() was a no-no, so thanks for pointing it out. Oh, and your example of a calendar function is really helpful, thanks so munch for that!

When I get those kinks out of it, would you mind looking it over (I'll post it in this thread), and giving further pointers? Your advice is really helpful.

Thanks again.

I'm glad to help. If there's some sort of cosmic coding karma, I'd like to be on the right side of it. =)

The logic in Days may well have been sound, and I was just having trouble following it. It seemed like for day you checked equivalence, then conditionally increment or decremented, but did both unconditionally for month and year (or something like that). I'm going from memory here, but that seemed to be the primary problem.

It may be reasonable to re-invoke main, but as I said, I had never seen it and it seems to make program flow a bit difficult to follow. In the example I posted i changed main and Days to return the number of days in a variable passed by reference, and used its return code to indicate success or failure instead. Then in main the return code was checked to see if there was success, and if not the loop would cycle to request input again. This would be my recommendation for handling something like this.

I'm sure I or others on the forum would be happy to review future revisions of the code. To make it easier for us, post future code examples in code tags, that look like: [CODE]...[/CODE]. This makes it much easier than opening a word document, turning it in to usable plaintext, etc.

-Lee
 
Code:
#include <stdio.h> 
#include <ctype.h> 
//JD= 367*2008 - ((7(2008 + (1)))/4) + 275*9/9 + 6 + 1721013.5
//JD= 736936 - 3515 + 275 + 6 + 1721013.5
//JD= 2454715.5
#define baseJD 2454716 //We will round up in all cases, assuming noon

int Days(int,int,int,int *); 
int printDayOfWeek(int); 
int printMonth(int); 
int makeJulian(int,int,int);

int main (int argc, char *argv[]) { 
  int d, m, y, ds;  
  int result=0;
  do {
    char ch='M'; 
    while (ch=='M'){ 
      printf("Enter the month (numerically).\n"); 
      scanf("%d", &m); 
      printf("Enter the day.\n"); 
      scanf("%d", &d); 
      printf("Enter the year.\n"); 
      scanf("%d", &y); 
      if(printMonth(m)) continue; 
      printf(" %d, %d is the date you want evaluated? Y=yes, N=no.\n",d,y); 
      while((toupper(ch)!='N')&&(toupper(ch)!='Y')) ch=getchar(); 
      ch=toupper(ch)-1; 
    } 
    result=Days(d,m,y,&ds);
  } while(result == -1);
  printf("%d/%d/%d ", m,d,y); 
  if(ds<0) { 
    printDayOfWeek(7-(-ds)%7); 
  } else { 
    printDayOfWeek(ds%7); 
  } 
  return 0;  
} 

int makeJulian(int year, int month, int day) { //Simplified by assuming date > 2/28/1900
  return 367*year - ((7*((month+9)/12))/4) + ((275*month)/9) + day + 1721014; //Assume noon, for ease
}

int Days (int day, int month, int year,int *numdays) {  
  short daysPerMonth[] = {0,31,(year%4==0 && year%100!=0)||year%400==0?29:28,31,30,31,30,31,31,30,31,30,31};
  *numdays=0; 

  if(day>daysPerMonth[month] || day < 1) { 
    printf("Invalid day\n\n"); 
    return -1;
  } 
  *numdays=makeJulian(year,month,day)-baseJD;
  return 0;
} 

int printDayOfWeek (int _d){ 
  char *nameOfDays[7] = {"Saturday","Sunday","Monday","Tuesday","Wednesday","Thursday","Friday"};
  if(_d < 0 || _d > 6) return 1;
  printf("%s\n",nameOfDays[_d]);
  return 0;
} 

int printMonth (int mo){ 
  char *nameOfMonths[12] = {"January","February","March","April","May","June","July","August","September","October","November","December"};
  if(mo < 1 || mo > 12) return 1;
  printf("%s",nameOfMonths[mo-1]);
  return 0; 
}
-Lee

This code doesnt do 11/10/1991.
it will do other days like 12/10/1991 Monday.:(

mind if i convert this code to Objective C and use it in my project? well some of it.
 
This code doesnt do 11/10/1991.
it will do other days like 12/10/1991 Monday.:(

mind if i convert this code to Objective C and use it in my project? well some of it.

I really can't speak to the reliability of this code. I was really just trying to demonstrate some things to the OP.

You are welcome to reuse this, but you should use NSCalendar to do this right if you have it available.

-Lee
 
I really can't speak to the reliability of this code. I was really just trying to demonstrate some things to the OP.

You are welcome to reuse this, but you should use NSCalendar to do this right if you have it available.

-Lee

This might be slightly OT, but does using non-primitives (NSNumber etc) avoid CPU issues? I don't have a PPC and Intel mac to try this out myself.
 
This might be slightly OT, but does using non-primitives (NSNumber etc) avoid CPU issues? I don't have a PPC and Intel mac to try this out myself.

Well... For endianness or width of primitives to be an issue you have to be doing unusual things that depend on byte order without proper handling (binary file or network IO), etc. or memcpy with a fixed width, and other such tricks. There are different issues going from big to little endian and 32 to 64 bit platforms, but the class of these problems almost always comes down to shenanigans or ignored warnings re: int to void * assignments, etc. Basically, you were doing something wrong or willfully unusual, but weren't made to pay for it with incorrect behavior on the original platform.

For these reasons, building/porting/testing between architectures is often a good way to catch errors that might not readily present themselves on a single target.

As for NSNumber saving you from said tomfoolery... probably not. All NSNumber does is wrap your primitive for use in collections or APIs that require an NSObject rather than a primitive. If you were doing something unseemly with an int before, and now you wrap it in an NSNumber, then pull it out of there before practicing your perversions, you've gained nothing. There are no operations performed on an NSNumber. There aren't methods like "leastSignificantByte" or "valueAsIntWithNetworkByteOrder" that might be affected. Whatever was being done without NSNumber will go on with it, there will just be extra steps to shove the value into and pull it out of the wrapper.

What will help is using NSUInteger instead of unsigned int, etc. so you know you're getting the same thing platform to platform. Otherwise, just use prototypes and treat warnings as errors.

-Lee
 
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.