looking for advice

[bbcat]

To practice C language,I began to develop a simple project which
implements the management of students' information.I have worked it for
one week. I plan to finish it for two weeks.
Are there some experienced friends who would like to read my source
code and give some advice to me.tks :)

---------------------------------------------------------
main.c
---------------------------------------------------------
#include <stdio.h>
#include <string.h>
#include "logic.h"

const char *filename = "student.dat";

main(void)
{
int menu_id = 99;
int student_num = 0;
struct node *phead = NULL;

void load_menu();
void func_menu_new(struct node *phead);// new a student
void func_menu_search(struct node *phead);
void func_menu_edit(struct node *phead);
void func_menu_print(struct node *phead);
void func_menu_exit(struct node *phead);

load_menu();
phead = init();

if(read_datafile(phead,filename) == 1)
{
printf(" The file %s doesn't exist! \n",filename);
}

student_num = get_student_num(phead);
printf(" there are %d students' information.\n",student_num);

tag_start: scanf("%d",&menu_id);

switch (menu_id)
{
case 0:
func_menu_exit(phead);
break;
case 1:
func_menu_new(phead);
break;
case 2:
func_menu_search(phead);
break;
case 3:
func_menu_edit(phead);
break;
case 4:
func_menu_print(phead);
break;
default:
printf("Sorry,Only 0--4 can be chosen,please input again :)\n");
}
if(menu_id !=0)
goto tag_start;

}

void load_menu()
{
clrscr();

printf("\n");
printf("\n");
printf(" Welcome to Use Student System \n");
printf(" =====================menu=====================\n") ;
printf(" Input a student's informations..................1\n");
printf(" Search student's informations...................2\n");
printf(" Modify student's informations...................3\n");
printf(" Print student's informations....................4\n");
printf(" exit system.....................................0\n");
printf("\n");
}

void func_menu_new(struct node *phead)
{
struct student *pnewstudent = (struct student *)(sizeof(struct
student));

printf("please input a new student's information\n");
scanf("%d",&(pnewstudent->id));
scanf("%s",pnewstudent->name);

student_add(phead,pnewstudent);

printf("you have input a student's informations successfully!\n");

free(pnewstudent);
pnewstudent = NULL;
}

void func_menu_print(struct node *phead)
{
struct node *ptemp;
int student_num;

ptemp = phead;
student_num = get_student_num(phead);
printf(" All Student Information\n");
printf(" =================================\n");
printf(" there are %d students' information.\n",student_num);
printf("| Student ID ");
printf("|");
printf(" Student name |\n");

while(ptemp->next != NULL)
{
printf("| %10d ",ptemp->next->stu_info.id);
printf("| %14s |\n",ptemp->next->stu_info.name);
ptemp = ptemp->next;
}
ptemp = NULL;
}

void func_menu_exit(struct node *phead)
{
if(save_to_datafile(phead,filename) == 0)
{
printf(" All students' informations are saved!");
}

free_memo(phead);
}

void func_menu_search(struct node *phead)
{
int search_type;
int search_id;
char search_name[20];
struct node *psearch;

printf("Search by what?\n");
printf(" 1: student ID\n");
printf(" 2: student name\n");
scanf("%d",&search_type);

while((search_type != 1) && (search_type != 2))
{
printf("sorry, please choose 1 or 2\n");
scanf("%d",&search_type);
}

switch (search_type)
{
case 1:
{
printf("please input the student's ID\n");
scanf("%d",&search_id);
psearch = search_by_id(search_id,phead);
if(psearch == NULL)
{
printf("sorry there are no student infomation with ID
=%d\n",search_id);
}
else
{
printf(" %d :",psearch->stu_info.id);
printf(" %s\n",psearch->stu_info.name);
}
}
break;
case 2:
{
printf("please input the student's name\n");
scanf("%s",search_name);
psearch = search_by_name(search_name,phead);

if(psearch == NULL)
{
printf("sorry there are no student infomation with name =
%d\n",search_name);
}
else
{
printf(" %d :",psearch->stu_info.id);
printf(" %s\n",psearch->stu_info.name);
}
}
break;
default:
printf("error occurs");
}
psearch = NULL;
}

void func_menu_edit(struct node *phead)
{
int stu_id,stu_id_new;
char stu_name_new[20];
struct node *psearch;

printf(" Please input the ID of the student you want to edit.\n");
scanf("%d",&stu_id);
psearch = search_by_id(stu_id,phead);

if(psearch == NULL)
{
printf("sorry there are no student infomation with ID
=%d\n",stu_id);
}
else
{
printf(" %d :",psearch->stu_info.id);
printf(" %s\n",psearch->stu_info.name);
}

printf(" Please input the student's new information!\n");
scanf("%d",&stu_id_new);
scanf("%s",stu_name_new);
psearch->stu_info.id = stu_id_new;
strcpy(psearch->stu_info.name,stu_name_new);

printf("You update a student's information successfully!\n");
psearch = NULL;
}


--------------------------------------------------------
logic.c
-------------------------------------------------------
#include "logic.h"
#include <stdio.h>
struct node * init()
{
struct node * ptemp;

ptemp = (struct node*)malloc(sizeof(struct node));
ptemp->next = NULL;

return ptemp;
}

void student_add(struct node *phead,struct student *pnewstudent)
{
struct node *pnew,*ptemp;

ptemp = phead;
pnew = (struct node*)malloc(sizeof(struct node));
pnew->stu_info.id = pnewstudent->id;
strcpy(pnew->stu_info.name,pnewstudent->name);

while(ptemp->next != NULL)
{
ptemp = ptemp->next;
}
ptemp->next = pnew;
pnew->next = NULL;
pnew = NULL;
ptemp = NULL;
}

int get_student_num(struct node *phead)
{
struct node *ptemp;
int student_num = 0;

ptemp = phead->next;

while(ptemp != NULL)
{
ptemp = ptemp->next;
student_num ++;
}
ptemp = NULL;
return student_num;
}

int save_to_datafile(struct node *phead,char *filename)
{
struct node *ptemp;
int student_num;
FILE *fp;

ptemp = phead;
student_num = get_student_num(phead);
fp=fopen(filename, "w");
fprintf(fp,"%d\n",student_num);

while(ptemp->next != NULL)
{
fprintf(fp, "%d", ptemp->next->stu_info.id);
fprintf(fp, "%s\n", ptemp->next->stu_info.name);
ptemp = ptemp->next;
}

ptemp = NULL;
fclose(fp);
return 0;
}

int read_datafile(struct node *phead,char *filename)
{
struct node *pnew,*ptemp;
FILE *fp;
int student_num;

ptemp = phead;
fp = fopen(filename, "r");

if(fp == NULL)
{
return 1;
}

fscanf(fp,"%d",&student_num);

while(student_num > 0)
{
pnew = (struct node*)malloc(sizeof(struct node));
fscanf(fp,"%d",&(pnew->stu_info.id));
fscanf(fp,"%s",pnew->stu_info.name);
ptemp->next = pnew;
ptemp = ptemp->next;
student_num --;
}
ptemp = NULL;
pnew = NULL;
fclose(fp);
return 0;
}

void free_memo(struct node *phead)
{
struct node *ptemp,*pnext;

ptemp = phead->next;

while(ptemp != NULL)
{
pnext = ptemp->next;
free(ptemp);
ptemp = pnext;
}

free(phead);
phead = NULL;
ptemp = NULL;
pnext = NULL;
}

struct node * search_by_id(int search_id,struct node *phead)
{
int id = search_id;
struct node *ptemp,*psearch;

psearch = NULL;
ptemp = phead->next;

while(ptemp != NULL)
{
if(ptemp->stu_info.id == id)
{
psearch = ptemp;
}
ptemp = ptemp->next;
}

return psearch;
}

struct node * search_by_name(char search_name[20],struct node *phead)
{
struct node *ptemp,*psearch;

psearch = NULL;
ptemp = phead->next;

while(ptemp != NULL)
{

if(strcmp(ptemp->stu_info.name,search_name) == 0)
{
psearch = ptemp;
}
ptemp = ptemp->next;
}

return psearch;
}

-----------------------------------------------------------
logic.h
-----------------------------------------------------
#ifndef LOGIC_H
#define LOGIC_H

#define LEN sizeof(struct student)

struct student
{
int id;
char name[20];
};

struct node
{
struct student stu_info;
struct node *next;
};

extern struct node * init();
extern int read_datafile(struct node *phead,char *filename);
extern int get_student_num(struct node *phead);
extern int save_to_datafile(struct node *phead,char *filename);
extern void student_add(struct node *phead,struct student
*pnewstudent);
extern void free_memo(struct node *phead);
extern struct node * search_by_id(int search_id,struct node *phead);
extern struct node * search_by_name(char search_name[20],struct node
*phead);
#endif

[Arthur J. O'Dwyer]

On Wed, 16 Mar 2005, bbcat wrote:

To practice C language,I began to develop a simple project which
implements the management of students' information.I have worked it for
one week. I plan to finish it for two weeks.
Are there some experienced friends who would like to read my source
code and give some advice to me.tks :)

Yes. His name is 'gcc', and you can find him online or on your hard
disk. Try

gcc -W -Wall -ansi -pedantic -O3 -c logic.c
gcc -W -Wall -ansi -pedantic -O3 -c main.c

You will find many trivial yet fatal errors (such as forgetting to
prototype 'malloc' before using it), as well as several errors that
are just plain dumb (such as passing a string to 'printf("%d")')
and several stylistic booboos (such as forgetting to write
'int main(void)' instead of just 'main(void)').
Fix the mistakes the compiler tells you, and then post again.
Oh, and try not to post so *MUCH*. Put the code in three separate
files on your Web site, and post the URL. Then people won't have to
cut and paste so much in order to help you. If you make it hard for
people to help you, they usually won't.

Finally, don't use hard tabs (ASCII character 0x09) in Usenet posts,
or in code you expect a lot of people to read. They can get horribly
mangled by Usenet, and also by text editors (some of which think one
tab is eight spaces, some four, some three, some two, and some even
weirder numbers). Run 'detab' on your source code before posting it.

-Arthur

[Thomas Matthews]

bbcat wrote:

To practice C language,I began to develop a simple project which
implements the management of students' information.I have worked it for
one week. I plan to finish it for two weeks.
Are there some experienced friends who would like to read my source
code and give some advice to me.tks :)

---------------------------------------------------------
main.c
---------------------------------------------------------
#include <stdio.h>
#include <string.h>
#include "logic.h"

const char *filename = "student.dat"; Do you want a const pointer to constant data?
const char * const filename = "student.dat";

main(void) The main() function returns an int. Declare it properly:
int main(void)
{
int menu_id = 99;
int student_num = 0;
struct node *phead = NULL;

void load_menu();
void func_menu_new(struct node *phead);// new a student
void func_menu_search(struct node *phead);
void func_menu_edit(struct node *phead);
void func_menu_print(struct node *phead);
void func_menu_exit(struct node *phead); Declarations don't need to be encapsulated inside a function.
A popular style is to place all the declarations before the
main() function. Some people even alphabetize the list.

I suggest you place the declarations into a header file
with explanations of what they do, and their return values,
if any.

load_menu();
phead = init();

if(read_datafile(phead,filename) == 1)
{
printf(" The file %s doesn't exist! \n",filename);
} Interesting, two points here:
1. A read failure or open failure does not indicate that the file
does not exist, only that the open failed. For example, the
file may exist, but your program doesn't have access rights to
open it.
2. The program announces the file doesn't exist but continues anyway.
Very bad karma.

student_num = get_student_num(phead);
printf(" there are %d students' information.\n",student_num);

tag_start: scanf("%d",&menu_id); Input from a user without a prompt.
More bad karma.
Label usage, could be eliminated. Depends on you allergy level
to gotos.

switch (menu_id)
{
case 0:
func_menu_exit(phead);
break;
case 1:
func_menu_new(phead);
break;
case 2:
func_menu_search(phead);
break;
case 3:
func_menu_edit(phead);
break;
case 4:
func_menu_print(phead);
break;
default:
printf("Sorry,Only 0--4 can be chosen,please input again :)\n");
}
if(menu_id !=0)
goto tag_start;

} Try this:
typedef void (*Menu_Function_Pointer)(struct node *);
struct Menu_Item
{
unsigned int value;
const char * prompt; /* or description */
Menu_Function_Pointer action;
};

const struct Menu_Item Main_Menu[] =
{
{0,
"exit system.....................................0\n",
func_menu_exit},
{1,
"Input a student's informations..................1\n",
func_menu_new},
{2,
"Search student's informations...................2\n",
func_menu_search}
};
const unsigned int Num_Main_Menu_Items =
sizeof(Main_Menu) / sizeof(Main_Menu[0]);

void load_menu(void)
{
static const char * Menu_Header =
"\n"
"\n"
" Welcome to Use Student System\n"
" =====================menu=====================\n";
unsigned int i;
fwrite(Menu_Header, sizeof(Menu_Header), 1, stdout);
for (i = 1; i < Num_Main_Menu_Items; ++i)
{
printf(" %s", Main_Menu[i].prompt);
}
printf("\n");
return;
}

unsigned int process_menu(struct * phead)
{
unsigned int selection;
unsigned int index;

do
{
scanf("%d", &selection);
for (index = 0; index < Num_Main_Menu_Items; ++index)
{
if (selection == Main_Menu[index].value)
{
(Main_Menu[index].action)(phead);
break; /* exit the "for" loop */
}
}
if (index >= Num_Main_Items)
{
printf("Sorry,Only 0--%d can be chosen,please input again :)\n",
Num_Main_Menu_Items - 1);
}
} while (index >= Num_Main_Items);
return selection;
}

void load_menu()
{
clrscr();

printf("\n");
printf("\n");
printf(" Welcome to Use Student System \n");
printf(" =====================menu=====================\n") ;
printf(" Input a student's informations..................1\n");
printf(" Search student's informations...................2\n");
printf(" Modify student's informations...................3\n");
printf(" Print student's informations....................4\n");
printf(" exit system.....................................0\n");
printf("\n");
}

[snip]

I suggest making your linked list into separate
functions so you can use it for other projects. ;-)

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.comeaucomputing.com/learn/faq/
Other sites:
http://www.josuttis.com -- C++ STL Library book
http://www.sgi.com/tech/stl -- Standard Template Library

[bbcat]

thank Thomas Matthews and Arthur J. O'Dwyer for your advice very much
:-)