Code Questions

This is a discussion on Code Questions within the C Programming forums, part of the General Programming Boards category; I have 2 different versions of code for a simple program. One version works and with minor changes after comparing ...

  1. #1
    Maybe? Maybe not...
    Join Date
    Feb 2005
    Location
    Atlanta
    Posts
    16

    Code Questions

    I have 2 different versions of code for a simple program. One version works and with minor changes after comparing line for line the two different version only have about 4 different lines. I have many questions about this as far as my style and optimization memory usage etc... But Im not sure if its cool to post the entire program (its about 100 lines maybe minus space for readability). My main question is one of them the bad one exits with a memory error I think and I cant figure out why these few changes make the program run fine (minus warnings which I could never figure out anyway. I dont need any help with the actual program as its for a class and its already completed. The questions are more for me than anything else.

  2. #2
    Registered User caroundw5h's Avatar
    Join Date
    Oct 2003
    Posts
    751
    take a chance and post your code - the bad one.. I'm sure someone around here would be able to help you out. Just make sure you list all the problems your having.

  3. #3
    Maybe? Maybe not...
    Join Date
    Feb 2005
    Location
    Atlanta
    Posts
    16
    This is the bad version. Im not sure (or else I wouldnt be asking) but I think it has something to do with scope or the way I am passing the structures. I messed up last night and accidentally started correcting this one so it doesnt exit with the exact same error but its close enough to baffle and concern me. Feel free to comment on any portion of it..its not an error its a learning opportunity.


    Code:
     
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    //Structure Definitions
    struct Employee{
    char FName[20];
    char LName[10];
    int HrsWorked;
    float PayRate;
    int StpFlag;
    float RPay;
    float OPay;
    float GPay;
    float FITAmount;
    float NPay;
    };
     
    struct Totals{
    float TRPay;
    float TOPay;
    float TGPay;
    float TFITAmount;
    float TNPay;
    };
     
    //Function Prototypes
    void OpenFiles(void);
    void CloseFiles(void);
    void PrintHeads(void);
    void PrintDetails(struct Employee);
    void PrintFooter(struct Totals);
    struct Totals ProcessLoop(struct Totals);
    struct Employee InputName(struct Employee);
    struct Employee InputDetails(struct Employee);
    struct Employee CalcPay(struct Employee);
    struct Employee CalcGPay(struct Employee);
    struct Employee CalcNPay(struct Employee);
    struct Employee CalcFIT(struct Employee);
     
    //Global Variables
    FILE *PFile;
     
    //Main Line
    int main()
    {
    struct Totals NTot = {0,0,0,0,0};
    OpenFiles();
    PrintHeads();
    NTot = ProcessLoop(NTot);
    PrintFooter(NTot);
    CloseFiles();
    return 0;
    }
    //Open Files
    void OpenFiles(void)
    {
    PFile = fopen("c:\datadump.txt","w");
    return;
    }
    //Module to print report headers
    void PrintHeads(void)
    {
    char HL1[] = "									 Weekly Payroll Report										 Page 01\n";
    char HL2[] = "														 2/15/05";
    char HL3[] = "\n\n	 Employee			 Hours		 Regular		Overtime		 Gross";
    char HL4[] = "\n	 Name				Worked		 Pay			 Pay			 Pay			 FIT			Net Pay\n\n";
    fprintf(PFile,HL1);
    fprintf(PFile,HL2);
    fprintf(PFile,HL3);
    fprintf(PFile,HL4);
     
    return;
    }
    //Main module for processing all data
    struct Totals ProcessLoop(struct Totals NTot)
    {
    struct Employee NEmp = {"","",0,0,0,0,0,0,0,0};
    NEmp = InputName(NEmp);
    while(NEmp.StpFlag != 1)
    {
    NEmp = InputDetails(NEmp);
    NEmp = CalcPay(NEmp);
    NTot.TRPay = NTot.TRPay + NEmp.RPay;
    NTot.TGPay = NTot.TGPay + NEmp.GPay;
    NTot.TNPay = NTot.TNPay + NEmp.NPay;
    NTot.TOPay = NTot.TOPay + NEmp.OPay;
    NTot.TFITAmount = NTot.TFITAmount + NEmp.FITAmount;
    PrintDetails(NEmp);
    system("cls");
    NEmp = InputName(NEmp);
    }
    return NTot;
    }
    //Get the Employee Names
    struct Employee InputName(struct Employee NEmp)
    {
    char TFName[4] = "";
    printf("Enter Employee Name\n");
    printf("\nFirst Name: ");
    scanf("%20s",NEmp.FName);
    fflush(stdin);
    for(int count = 0;count <= 4;count++)
    TFName[count] = tolower(NEmp.FName[count]);
    if(strcmp(TFName,"stop") != 0)
    {
    printf("Enter Last Name: ");
    scanf("%10s",NEmp.LName);
    fflush(stdin);
    strcat(NEmp.FName," ");
    strcat(NEmp.FName,NEmp.LName);
    system("cls");
    }
    else
    {
    NEmp.StpFlag = 1;
    }
     
    return NEmp;
    }
    //Input Employee Details
    struct Employee InputDetails(struct Employee NEmp)
    {
    printf("Enter Employee Details\n");
    printf("\nEnter Hours Worked: ");
    scanf("%d",&NEmp.HrsWorked);
    fflush(stdin);
    printf("Enter Hourly Pay: ");
    scanf("%f",&NEmp.PayRate);
    fflush(stdin);
    return NEmp;
    }
    //Main Module to calculate pay and tax
    struct Employee CalcPay(struct Employee NEmp)
    {
    if(NEmp.HrsWorked > 40)
    {
     
    NEmp.RPay = 40 * NEmp.PayRate;
    NEmp.OPay = (NEmp.HrsWorked - 40) * (NEmp.PayRate * 1.5);
    }
    else
    {
    NEmp.RPay = NEmp.HrsWorked * NEmp.PayRate;
    NEmp.OPay = 0;
    }
    NEmp = CalcGPay(NEmp);
    NEmp = CalcFIT(NEmp);
    NEmp = CalcNPay(NEmp);
     
    return NEmp;
    }
    struct Employee CalcFIT(struct Employee NEmp)
    {
    const float FITRate = .15;
    NEmp.FITAmount = NEmp.GPay * FITRate;
    return NEmp;
    }
    struct Employee CalcGPay(struct Employee NEmp)
    {
    NEmp.GPay = NEmp.RPay + NEmp.OPay;
     
    return NEmp;
    }
    struct Employee CalcNPay(struct Employee NEmp)
    {
    NEmp.NPay = NEmp.GPay - NEmp.FITAmount;
    return NEmp;
    }
    void PrintFooter(struct Totals NTot)
    {
    char FL1[] = "\n\n						 Totals:	 ";
    fprintf(PFile,FL1);
    fprintf(PFile,"$ %8.2f",NTot.TRPay);
    fprintf(PFile,"	 $ %8.2f",NTot.TOPay);
    fprintf(PFile,"	 $ %8.2f",NTot.TGPay);
    fprintf(PFile,"	 $ %8.2f",NTot.TFITAmount);
    fprintf(PFile,"	 $ %8.2f",NTot.TNPay);
    return;
    }
    void CloseFiles(void)
    {
    fclose(PFile);
    return;
    }
    void PrintDetails(struct Employee NEmp)
    {
    fprintf(PFile," %-20s",NEmp.FName);
    fprintf(PFile,"		%2d",NEmp.HrsWorked);
    fprintf(PFile,"		$ %8.2f",NEmp.RPay);
    fprintf(PFile,"	 $ %8.2f",NEmp.OPay);
    fprintf(PFile,"	 $ %8.2f",NEmp.GPay);
    fprintf(PFile,"	 $ %8.2f",NEmp.FITAmount);
    fprintf(PFile,"	 $ %8.2f\n",NEmp.NPay);
    return;
    }

  4. #4
    Registered User caroundw5h's Avatar
    Join Date
    Oct 2003
    Posts
    751
    what exactly are your errors that you are having problems with. Your code compiled fine with devC++ on my box - with an exception.
    line 102
    Code:
    for(int count = 0;count <= 4;count++)
    this line makes your code less portable. you can do this in c99 i believe but not before. I simply declared int count oustside of the loop.

    The other problems i see that others will surely comment on is using fflush(stdin) bad - as this is undefined.

    You have a whole bunch of functions return type struct, which is just a waste of memory, cause your creating a copy of the struct with each call to the functions. See if you can't put a pointer to struct in their and do what needs to be done to the struct - its much more efficient and easiear on memory.

    system("cls") isn't portable either. and calling system is bad as well. Search the faq and the board for why, they are constantly covered.

    and as well you don't check to maker sure you can actually open a file, or that the file is actually closed properly. this line
    Code:
    PFile = fopen("c:\datadump.txt","w");
    should be
    Code:
    PFile = fopen("c:\\datadump.txt","w");
    otherwise you get an unknown escape sequence and you might get the creation of your file, but not where you expect.

    also
    Code:
    char HL1[] = "                                     Weekly Payroll Report                                         Page 01\n";
    char HL2[] = "                                                         2/15/05";
    char HL3[] = "\n\n     Employee             Hours         Regular        Overtime         Gross";
    char HL4[] = "\n     Name                Worked         Pay             Pay             Pay             FIT            Net Pay\n\n";
    I'd use some field width to display it correctly in the file. As it stands the file output is kinda hodepdge and all over the place. Have you tested this? check it out.
    Last edited by caroundw5h; 02-21-2005 at 05:01 PM. Reason: call attention to change
    Warning: Opinions subject to change without notice

    The C Library Reference Guide
    Understand the fundamentals
    Then have some more fun

  5. #5
    Slave MadCow257's Avatar
    Join Date
    Jan 2005
    Posts
    735
    Since you want to know about your style, I would recommend using good formatting. Tabs make everything so much easier to read, especially when dealing with nested loops.

  6. #6
    Maybe? Maybe not...
    Join Date
    Feb 2005
    Location
    Atlanta
    Posts
    16
    I just realized that I didnt post the changes and as far as the output I lined it up in notepad in windows. The first line throws it off because part of the project reqs are that the authors name goes in the first line of the report on the top left. I dont post any identifiable information over any internet medium Im sure you understand my logic there. Changes I will post in a second with the edit in this post.

    here are the changes only

    Code:
     
    //Structure Definitions
    struct Employee{
    char FName[20];
    char LName[10];
    char TFName[4];
    int HrsWorked;
    float PayRate;
    float RPay;
    float OPay;
    float GPay;
    float FITAmount;
    float NPay;
    int StpFlag;
    };
     
    //Main module for processing all data
    struct Totals ProcessLoop(struct Totals NTot)
    {
    struct Employee NEmp = {"","","",0,0,0,0,0,0,0,0};
    NEmp = InputName(NEmp);
    while(NEmp.StpFlag != 1)
    {
    NEmp = InputDetails(NEmp);
    NEmp = CalcPay(NEmp);
    NTot.TRPay = NTot.TRPay + NEmp.RPay;
    NTot.TGPay = NTot.TGPay + NEmp.GPay;
    NTot.TNPay = NTot.TNPay + NEmp.NPay;
    NTot.TOPay = NTot.TOPay + NEmp.OPay;
    NTot.TFITAmount = NTot.TFITAmount + NEmp.FITAmount;
    PrintDetails(NEmp);
    system("cls");
    NEmp = InputName(NEmp);
    }
    return NTot;
    }
     
    //Get the Employee Names
    struct Employee InputName(struct Employee NEmp)
    {
     
    printf("Enter Employee Name\n");
    printf("\nFirst Name: ");
    scanf("%20s",NEmp.FName);
    fflush(stdin);
    for(int count = 0;count <= 4;count++)
    NEmp.TFName[count] = tolower(NEmp.FName[count]);
    if(strcmp(NEmp.TFName,"stop") != 0)
    {
    printf("Enter Last Name: ");
    scanf("%10s",NEmp.LName);
    fflush(stdin);
    strcat(NEmp.FName," ");
    strcat(NEmp.FName,NEmp.LName);
    system("cls");
    }
    else
    {
    NEmp.StpFlag = 1;
    }
     
    return NEmp;
    }
    These are all from the good code. One of the warnings was an unknown escape at the c:\datadump.txt which is just the file I use on my machine to capture the output without having to print it that gets changed to the network printer when its all done.

    conversion from double to float right here
    Code:
    struct Employee CalcPay(struct Employee NEmp)
    {
    if(NEmp.HrsWorked > 40)
    {
     
    NEmp.RPay = 40 * NEmp.PayRate;
    NEmp.OPay = (NEmp.HrsWorked - 40) * (NEmp.PayRate * 1.5); /here for the warning
    }
    else
    {
    NEmp.RPay = NEmp.HrsWorked * NEmp.PayRate;
    NEmp.OPay = 0;
    }
    NEmp = CalcGPay(NEmp);
    NEmp = CalcFIT(NEmp);
    NEmp = CalcNPay(NEmp);
     
    return NEmp;
    }
    and this line gives me c4305 initializing truncation const double to const float
    Code:
     
    const float FITRate = .15;
    the first error Ive seen many times and havent really looked at since my test data always came out fine second pops up less often but same effect.

    when I run the first version without and changes it will take all the data fine until I signal stop which is when it throws a windows error and asks me to debug which i do and the first thing it shows is

    unhandled exception in payroll.exe:0xc0000005 Access Violation

    then it throws me to the processloop return line where it returns NTot. NTot is the structure holding my totals right now and feeds them to the module that prints footers and final lines.

    As far as all the passing. I was considering using three different structures but that made even less sense. Would it be better to not use a structure here or create smaller local variables and use only them or create the two strucutres but use the smaller cariables in the functions and use those as return values to fill the structure? Im just really not used to only being able to return one value at a time. I knew it looked strange and I was going to post it a few days ago I figured I would finish it with a few changes and then post the results with questions for comments
    Last edited by Taikon; 02-21-2005 at 05:52 PM. Reason: Missing info in the first post

  7. #7
    Registered User
    Join Date
    Mar 2004
    Posts
    536
    Quote Originally Posted by Taikon
    when I run the first version without and changes it will take all the data fine until I signal stop which is when it throws a windows error and asks me to debug which i do and the first thing it shows is
    Well, since the error is triggered by "stop", look at the code around "stop"

    Code:
    struct Employee InputName(struct Employee NEmp)
    {
    char TFName[4] = "";
    printf("Enter Employee Name\n");
    printf("\nFirst Name: ");
    scanf("%20s",NEmp.FName);
    fflush(stdin);
    for(int count = 0;count <= 4;count++)
    TFName[count] = tolower(NEmp.FName[count]);
    if(strcmp(TFName,"stop") != 0)
    You have TFName[4], so the array elements have index 0, 1, 2, 3. Then the loop goes from 0 to 4. Furthermore there is no guarantee that the string you are giving to strcmp is teriminated with a zero byte anywhere within your legal address space.

    Maybe you could try something like this;

    Code:
    char TFName[5] = {0}; /* initialize to all zeros */
    for (count = 0; count < 4; count++) {
      TFName[count] = tolower(......)
    Now, you have four characters followed by a '\0', so that strcmp won't throw a hissy fit (access violation).

    By the way: I am shocked that none of the fflush(stdin) commandos has excoriated you about this despicable practice yet. My advice: don't use fflush(stdin). Even if it appears to work on your system, you shouldn't depend on it, since it isn't standard. There are other, standard, ways of emptying the input buffer when you really need to.

    You really should get into the habit of always (always) checking return values from fopen(), scanf(), etc.

    Regards,

    Dave
    Last edited by Dave Evans; 02-22-2005 at 03:42 PM.

  8. #8
    Registered User computerfreaks's Avatar
    Join Date
    Jan 2005
    Posts
    30
    A good tip..... when you have a problem and you are unsure where it is coming from, put some debug lines in the code.

    The best way (I find personaly) to do this is to get the program to print "Debug 1", followed by a pause, then "debug 2" etc at different points in the program (if you know a certain part is executing fine then don't bother putting them there). When you run the program you will then know which parts have executed without a problem.

    This serves to be very usefull as you can narrow the problem right down to a line sometimes and then i is A LOT easier to fix.

  9. #9
    Maybe? Maybe not...
    Join Date
    Feb 2005
    Location
    Atlanta
    Posts
    16

    Question

    Sorry about bumping this but the question is along the same line and the program code already posted is a good example.

    The question is about the structures and the variables Ive stored in them. I really can't explain why I have been doing it like this lately aside from experimenting with what you can and can't do with a structure. I understand why you wouldn't want to do this on almost any program thats used in a business enviornment or code it this way for a job as far as how I have passed the entire structure each time.

    I guess I will phrase the actual question like this:
    By convention or good habit how WOULD you want to store these variables?

    It seems like these might be the best way to make updating the program for someone coming in behind me fairly easy. Im also thinking that I might have gotten a different response if I had passed only one variable or passed by pointer as well, although I am not exactly sure you can pass just one variable and I think I remember from experimenting that it caused some problems I had trouble pinpointing and fixing.

    If this comes off as a confused ramble my apologies its still fairly early for me, and again sorry for the bump.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. C interview questions
    By natrajdreams in forum C Programming
    Replies: 7
    Last Post: 12-12-2010, 12:40 PM
  2. Replies: 23
    Last Post: 04-20-2009, 08:35 AM
  3. Binary Search Trees Part III
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 16
    Last Post: 10-02-2004, 04:00 PM
  4. Problem : Threads WILL NOT DIE!!
    By hanhao in forum C++ Programming
    Replies: 2
    Last Post: 04-16-2004, 02:37 PM
  5. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 10:33 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21