Solved

creating a function, then move the prototype and function to seperate header and source files

Posted on 2004-10-11
16
218 Views
Last Modified: 2010-04-15
Hi,
I'm new to C programming and attempting to create a program to extract data fields from a string of characters, the fields are seperated by commas.  The commas do not appear in the printed output, they just act as delimiters and the result has to be printed out in a certain format.

So far I have to code displayed below, which does work.  But now I want to move the code into a function (called displayUnit) which takes in two arguements(a text string and a Unit) and then sort out my program so that i have a main.c file, a functions.h file with my prototypes in and a functions.c file with my functions in it.  This is where I'm getting stuck as I can seem to make it work after moving everything about into different functions etc!
as i said i'm very new to c so any advise, however basic, would be greatly appreciated.
thanks in advance

#include <stdio.h>
#include <stdlib.h>

typedef struct
{
      char unitCode[9];
      char unitTitle[100];
      char slot;
      int enrolments;
}Unit;

int main (void)
{      
                char *comma;
      char test[] = "ght75757,maths unit,d,80";
      Unit u;
      
        comma = strtok(test, ",");
        strcpy(u.unitCode, comma);

        comma = strtok(NULL, ",");
        strcpy(u.unitTitle, comma);

        comma = strtok(NULL, ",");
        u.slot = comma[0];

       comma = strtok(NULL, ",");
       u.enrolments = atoi(comma);

       printf("%s:%s\nSlot:%d Enrolments:%d\n",u.unitCode,u.unitTitle,u.slot,u.enrolments);
      
}
0
Comment
Question by:louisebe
  • 5
  • 5
  • 4
  • +1
16 Comments
 
LVL 45

Accepted Solution

by:
Kdo earned 125 total points
ID: 12275958
Hi louisebe,

I see a couple of problems.  

1)  Never parse a literal.  Never.  Literals are often kept in read-only memory so when strtok() goes to write the token terminator (zero) into the string your program may abort for a memory access violation.

char *comma;
char test[] = "some string";
char *ParsedString;

ParsedString = strdup (test);

comma = strtok (ParsedString, ",");

etc....


2)  Moving the parsing logic to a function is pretty easy.  I'd suggest a function with two parameters -- the string to parse and the record to be populated.

int CheckLine (char *String, Unit *DataRecord)
{
  char *Parameter;
  char *NextParameter;

  comma = strtok (String, ",");
  strcpy (DataRecord->unitCode, comma);

  comma = strtok (NULL, ",");
etc....
}

main ()
{
  char test[] = "some string";
  char *ParsedString;
  Unit u;

  ParsedString = strdup (test);
  CheckLine (ParsedString, &u);
}

etc...


Good Luck,
Kent
0
 
LVL 1

Expert Comment

by:manojantony
ID: 12277147

try this ....

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct
{
     char unitCode[9];
     char unitTitle[100];
     char slot;
     int enrolments;
}Unit;

void splitLine(Unit, char *);

int main (void)
{    
     char test[] = "ght75757,maths unit,d,80"; /* test string */
     Unit u;
     splitLine(u, test); /* function to split string */
     
}
/* function to split a line of string */

void splitLine(Unit u, char *line)
{
            char *word, *pErr;
            int count = 0;
            const char * sep = ",\0\n" ; /* seperators....*/
            for (word = strtok(line, sep); word; word = strtok(NULL, sep))
            {
         switch (count)
                   {
                           case 0: /* example for a string value */
                                  strcpy(u.unitCode, word);
              break;
                  case 1:
                /*.......*/
                            case 2:
                                    /*.......*/
                            case 3:
                                         /* example for an integer value */
                   u.enrolments = strtol(word, &pErr, 0);  
                   break;      
            }
               count++;
       }      
      printf("%s %s %s %d",u.unitCode, u.unitTitle, u.slot, u.enrolments);
}
0
 
LVL 7

Expert Comment

by:ravs120499
ID: 12278133
As Kdo said, it's not a good idea to parse a string literal directly. But just getting down to your question, here is how you can split them into source and header files.

First, I will recommend that you define the struct Unit in another header file, day my_types.h You can do it in functions.h itself without any problem, but for reasons I will not go into here, I recommend having a different file.

So, here is what you have:

my_types.h
-------------
typedef struct
{
 ...
} Unit;


functions.h
------------
/* Declare the function prototypes */

/*
 * parseUnit accepts a string, parses it into a Unit. It expects to be passed a pointer to a valid Unit structure
 * which it will populate with the parsed string. Another solution is for parseUnit() to internally allocate a Unit
 * struct and return it as the return value of the function. However, that requires you to be comfortable with
 * dynamic memory allocation and pointers, so is probably a bit further down the line.
 */
extern void parseUnit(char *text, Unit *p_unit);

/*
 * displayUnit accepts a Unit struct and prints it to console.
 * A neater solution is for displayUnit to accept a pointer to Unit struct, rather than the struct itself.
 * That will reduce the amount of data that needs to be copied into the stack before calling the function. Again, that requires
 * familiarity with pointers.
 */
extern void displayUnit(Unit unit);

functions.c
------------
#include <stdio.h>
/* etc. */
#include "my_types.h"

/* No need to include functions.h */

void parseUnit(char *text, Unit *p_unit)
{
        char *comma;
     
        comma = strtok(text, ",");
        strcpy(p_unit->unitCode, comma);

       /* and so on for the other fields */

       return;
}

void displayUnit(Unit unit)
{
     printf("%s:%s\nSlot:%d Enrolments:%d\n", unit.unitCode, unit.unitTitle, unit.slot, unitenrolments);
     return;
}

main.c
-------

#include <stdio.h>
/* etc */
#include "my_types.h"
#include "functions.h"

void main(int argc, char **argv)
{
  char *text = "ght75757,maths unit,d,80";
  Unit unit;

  parseUnit(text, &unit);
  displayUnit(unit);  
  return;
}

0
 
LVL 45

Expert Comment

by:Kdo
ID: 12281503
Hi manojantony,

I think I'd take a slightly different direction.  Your method is very workable and quite common, but it gets kind of long.

#define MAX_PARAMETERS 20

void splitLine(Unit u, char *Source)
{
  char *List[MAX_PARAMETERS+1];
  char *Line;
  char *Parameter;
  int    Count;

  memset (List, 0, sizeof (char *) * (MAX_PARAMETERS + 1));
  if (*Source)
  {
    Count = 0;
    Line = strdup (Source);
    List[Count++] = strtok (Line, ",");
    while (Count < MAX_PARAMETERS)
      if ((Parameter = strtok (NULL, ",")) != NULL)
        List[Count++] = Parameter;
      else
        break;
    strncpy (u->unitCode, List[0], MAX_UNIT_CODE_LENGTH);
    strncpy (u->unitTitle, List[1], MAX_UNIT_TITLE_LENGTH);
    ...
    free (Line);
  }

Of course, this technique will probably get you talked about in a beginning C class.  :)


Kent
0
 

Author Comment

by:louisebe
ID: 12284938
hi,
thanks all for your help.  Im new to experts exchange and im amazed at the level and speed of help, its excellent! thank u so much, i will definately use this site again!
i have managed to split my program into a function and main method, and it still works.  next step is to put them in different files, with the information given here, im quite confident i know how to do that now. ive also got to change my program so that it takes in a line from a text file instead of just a string, but now ive got the basics working, as i said before, i feel much more confident now.
thanks again for assistance!
0
 

Author Comment

by:louisebe
ID: 12309513
sorry to be a pain but i have another enquiry!! is it ok to put it here or should i be opening another question...?! anyways, i'll put it here and see what happens!
using my program stated in my very first post, i now need to create a function which reads in lines from a text file(instead of just from a string), and passes them to an array of 'unit' structures.  so that i can then print out the file in the specified format.
any help would be greatly appreciated as ive spent hours trying different methods and seem to be getting no-where!
0
 
LVL 1

Expert Comment

by:manojantony
ID: 12309678
hi,
Can I see what are the methods you have tried ?
0
Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 
LVL 1

Expert Comment

by:manojantony
ID: 12309781
void
loadData(Unit u)
{
    FILE           *fp;                                /* file pointer */
    if ((fp = fopen("filename.txt", "r")))    /* open for read */
    {
         printf("Reading data files...\n");
         while ((fgets(line,MAX_LEN, fp) != NULL))
         {
            /* do the parsing and reading of data here */  
          }
          printf("structure is loaded with data from file\n");
         fclose(fp);
    }
     else
          printf("Error in opening file\n");
}
0
 

Author Comment

by:louisebe
ID: 12310014
hiya, thanks for replying,ive tried modifying my original code. for example -

(fp = the file)
while(!feof(fp))
{
 ++numUnits;
 unitList = (Unit*)realloc( unitList, sizeof(Unit) * numUnits);
 fgets(inputLine, sizeof(inputLine),fp);

code = strtok(inputLine, ",");
strcpy( unitList[numUnits-1].unitCode , code);

...same for title,slot etc.
}
so adding in an array called numunits, so attemting to put the results in an array. thats all ive got to doing really.  because that doesnt seem to work, i havent done actually printing it yet.  im really struggling to get my head around c, i get what i need it to do, but actually doing it, is a different story!!thanks for your help so far anyways
0
 

Author Comment

by:louisebe
ID: 12310405
Ive possibly had a bit of a breakthrough...it does something then comes up with an error and cant continue. this is what ive got as my readdata function - (i then call it from main and it does run a print out a few units in the correct structure before finding an error)

int ReadData(Unit *unit)
{
      char *code;
      char *title;
      char *slot1;
      char *enrol;
      char inputline[500];
      int numunits = 0;
    FILE *fp = fopen("units.txt","r");
      int i=0;

      unit = NULL;
      while(!feof(fp))
      {
      numunits++;
      unit = (Unit*)realloc(unit,sizeof(Unit)*numunits);
      fgets(inputline,sizeof(inputline),fp);
      
      code = strtok(inputline, ",");
      strcpy (unit[numunits-1].unitCode, code);
            
      title = strtok (NULL, ",");
      strcpy (unit[numunits-1].unitTitle, title);
            
      slot1 = strtok (NULL, ",");
      unit[numunits-1].slot = slot1[0];
      
      enrol = strtok (NULL, ",");
      unit[numunits-1].enrolments = atoi(enrol);
      
            while(enrol = strtok(NULL,","))
            {
                  numunits++;
                  unit = (Unit*)realloc(unit,sizeof(Unit) *numunits);
            }
      
      for(i=0;i<numunits;i++)
      printf("%s:%s\nSlot:%c Enrolments:%d\n",unit[i].unitCode,unit[i].unitTitle,unit[i].slot,unit[i].enrolments);
      printf("there are %d units in the file",numunits);
            
      fclose(fp);
      }
      return 1;
      
}


am i completely off track or can it be fixed?! again, any help greatly appreciated.
0
 
LVL 45

Expert Comment

by:Kdo
ID: 12310517
Hi louisebe,

I'm confused about your inner while() loop.  You increment numunits for every trailing comma, but don't put anything into the structures at those indexes.  This section should probably be deleted or at least you should initialize the records.

Also, just in case the data doesn't match the descriptions in the structure, use strncpy() instead of strcpy().


Kent
0
 
LVL 1

Expert Comment

by:manojantony
ID: 12310574
I agree with Kent

Also .. where is the end of the outer while loop ??
Try printing the values in another function PrintData(... , ...)

MA
0
 

Author Comment

by:louisebe
ID: 12318354
hey,
i have changed the funciton a bit and got rid of the inner while loop, it was something i'd tried earlier and forgotten to get rid of it!! it does now work but i dont seem to have enough print out space, it misses about the first 15 units before it starts printing.  any ideas why this is?
also i dont really get the point system on this website, have i awarded some points? should i be awarding more as u kdo+manojantony have helped me loads? if i need to give me points, let me know how and i'll do it as i dont expect u to help me for free!!
this is my updated function -
int ReadData(Unit *unit)
{
      char *code;
      char *title;
      char *slots;
      char *enrol;
      char tempstring[500];
      int numunits=0;
    int i=0;

      FILE *fp = fopen("CMPunits.txt","r");

      unit = NULL;
      while(!feof(fp))
      {
            numunits++;
            unit = malloc(sizeof(Unit)*numunits);
            fgets(tempstring,500,fp);

            code = strtok(tempstring, ",");
            strcpy (unit->unitCode, code);
            
            title = strtok (NULL, ",");
            strcpy (unit->unitTitle, title);
            
            slots = strtok (NULL, ",");
            unit->slot = slots[0];
      
            enrol = strtok (NULL, ",");
            unit->enrolments = atoi(enrol);
      
            printf("%s:%s\nSlot:%c Enrolments:%d\n\n\n",unit[i].unitCode,unit[i].unitTitle,unit[i].slot,unit[i].enrolments);
            
      }
      fclose(fp);
      return numunits;
}
0
 
LVL 45

Expert Comment

by:Kdo
ID: 12318408

Easy one.  :)

Within the loop, you need print out the item that you just created.

Or, a simpler approach would be to use two loops.  Take the print statement out of this while() loop and put it in its own loop after the while() loop.


Kent
0
 
LVL 45

Expert Comment

by:Kdo
ID: 12318492

I guess that wasn't very helpful, was it?

Within the loop, you always store the data at unit->Element.  Yet every time through the loop you expand the array.  I suspect that you want to store the data at unit[i]->Element and increment *i* after you finish defining the unit.


Kent
0

Featured Post

How to improve team productivity

Quip adds documents, spreadsheets, and tasklists to your Slack experience
- Elevate ideas to Quip docs
- Share Quip docs in Slack
- Get notified of changes to your docs
- Available on iOS/Android/Desktop/Web
- Online/Offline

Join & Write a Comment

Preface I don't like visual development tools that are supposed to write a program for me. Even if it is Xcode and I can use Interface Builder. Yes, it is a perfect tool and has helped me a lot, mainly, in the beginning, when my programs were small…
This tutorial is posted by Aaron Wojnowski, administrator at SDKExpert.net.  To view more iPhone tutorials, visit www.sdkexpert.net. This is a very simple tutorial on finding the user's current location easily. In this tutorial, you will learn ho…
The goal of this video is to provide viewers with basic examples to understand and use structures in the C programming language.
The goal of this video is to provide viewers with basic examples to understand opening and reading files in the C programming language.

746 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

12 Experts available now in Live!

Get 1:1 Help Now