C++

Forum

 
Criticize my code please

kusen (3)
I created this simple database(written in C) in order to see how structures worked, and also to see how much I remembered about coding in general.

also, There is no ability to save the data, as I haven't gotten that far and it was never really intended to be in the code to start with.

I'm especially interested in:

files - If I'm using too many different files(or too few). Such as, maybe the structure would be better of added into another header, or even use the structure header for both definitions and structure. That sort of thing.

Comments - If I'm using too few, or too many comments, and\or where they would be better placed, and if their even readable.
kusen (3)
Code:

Main.c 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
 
#include <stdio.h>
#include <stdlib.h>

#include "Structures.h" // Contains the database structure
#include "Functions.h" // Contains the database functions

int main()
{
   database data[100];

   char input[10];

   int menu = 50;
   int id = 0;

   printf("Welcome to your new movie database!\n");

   while(TRUE)
   {
      printf("\nMain menu:\n");
      printf("0 - Exit\n");
      printf("1 - See the whole database\n");
      printf("2 - Search trough the database\n");
      printf("3 - Add a new movie to the database\n");
      printf("4 - Delete and item from the database\n");

      while(TRUE)
      {
         printf(">> ");
         gets(input);
         if(sscanf(input,"%d", &menu))
            break;

         printf("Please only input a number!");
      }

      switch(menu)
      {
         case 0:
            return 0;

         case 1:
            data_View(id, data);
            break;

         case 2:
            data_Search(id, data);
            break;

         case 3:
            data_Add(&id, data);
            break;

         case 4:
            data_Del(&id, data);
            break;

         default:
            printf("Please enter one of the meny items\n");

      }// end switch
   }// end main loop
} // end main


Structures.h

1
2
3
4
5
6
7
8
9
10
 
#ifndef HEADER_FILE_H
#define HEADER_FILE_H

typedef struct database {
   char movie[50];
   int  year;
   int  id;
}database;

#endif


Functions.h 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
#define TRUE 1
#define FALSE 0

#ifndef HEADER_FILE_H
#define HEADER_FILE_H

#include "Structures.h" // Contains the database structure


void data_View(int id, database data[100]);

data_Search(int id, database data[100]);

data_Add(int id, database data[100]);

data_Del(int *id, database data[100]);

#endif


Functions.c 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
 
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

#include "Structures.h" // Contains the database structure
#include "Functions.h"

#define TAB_WIDTH 45
#define TOTAL_LENGHT 62
#define STR_LENGHT 50

data_View(int id, database data[100])
{
   int i;
   int j;
   int len;

   printf("title %*s| year | id\n",TAB_WIDTH,"");
   for(i = 0; i < id; i++)
   {
      for(j = 0; j < TOTAL_LENGHT; j++)
         printf("-");

      len = (STR_LENGHT - strlen(data[i].movie));
      printf("\n%s %*s| %d | %02d\n",data[i].movie,len,"", data[i].year, data[i].id);
   }

      printf("\n");

} // end function data_View

data_Search(int id, database data[100])
{
   char input[10];

   int menu = 50;
   int i;
   int j;
   int len;

   bool true_Flag = FALSE;
   bool false_Flag = FALSE;

   if(!id)
   {
      printf("You don't have any items in your database!\n");
      return;
   }

   while(TRUE)
   {
      printf("\n\nMenu\n");
      printf("0 - Back\n");
      printf("1 - Search from movie title\n");
      printf("2 - Search from year\n");
      printf(">> ");
      while(TRUE)
      {
         gets(input);
         if(sscanf(input,"%d",&menu))
            break;

         printf("Please input a number!\n");
      }

      switch (menu)
      {

         case 0:
            printf("\n");
            return;

         case 1:
         {
            char search[50];
            char movie_Title[50];
            const char delimiters[] = " .,;:!-";
            char *string_Fragment;

            false_Flag = FALSE;
            printf("Please enter movie title you wish to search for\n");
            printf("[case-sensetive and needs to contain the entire title!]\n");
            printf(">> ");
            gets(search);
            printf("title %*s| year | id\n",TAB_WIDTH,"");
            for(i = 0; i < id ; i++)
            {
               strcpy(movie_Title,data[i].movie);
               string_Fragment = strtok(movie_Title,delimiters);
               if(!stricmp(string_Fragment, search))
               {
                  true_Flag = TRUE;
               }

               while(!string_Fragment)
               {
                  string_Fragment = strtok(NULL,delimiters);
                  if(!stricmp(string_Fragment,search))
                  {
                     false_Flag = TRUE;
                     true_Flag = TRUE;
                     break;
                  }
               }


               if(true_Flag)
               {
                  len = STR_LENGHT - strlen(data[i].movie);
                     false_Flag = TRUE;
                  for(j = 0; j < TOTAL_LENGHT; j++)
                     printf("-");
                  printf("\n%s %*s| %d | %02d\n",data[i].movie,len,"", data[i].year, data[i].id);
               }

               true_Flag = FALSE;

            }
            if(!false_Flag)
               printf("Couldn't find anything with searchword: %s",search);

            break;

         } // end case 1 scope



         case 2:
         {
            int search;

            printf("Please enter the year the movie was released\n");
            printf(">> ");
            while(TRUE)
            {
               gets(input);
               if(sscanf(input, "%d", &search))
                  break;
               printf("Please only input a number!");
            }

            for(i = 0; i < id; i++)
            {
               if(search == data[i].year)
               {
                  false_Flag = TRUE;
                  len = STR_LENGHT - strlen(data[i].movie);
                  for(j = 0; j < TOTAL_LENGHT; j++)
                     printf("-");
                  printf("\n%s %*s| %d | %02d\n",data[i].movie,len,"", data[i].year, data[i].id);
               }
            }

            printf("\nflag=%d\n",false_Flag);
            if(!false_Flag)
               printf("couldn't find: \"%d\"\n",search);

            break;

         } // end case 2 scope

         default:
            printf("Please select a valid menu option!\n");

      } // end switch scope
   } // end main loop
} // End function data_Search

data_Add(int *id, database data[100])
{
   char input[10];
   char movie[50];

   int year;
   int i;

   printf("\nPlease enter the name of the movie[max 50 characters][0 to cancel]\n");
   while(TRUE)
   {
      printf(">> ");
      if(strlen(gets(movie)) < 50)
      {
         if(!strcmp(input,"0"))
            return;

         break;
      }

      printf("\nPlease enter a name of less then 50 characters!\n");
      printf("You had #%d of characters!\n", strlen(movie));
   }

   printf("\nPlease enter the year the movie was produced[4digits][0 to cancel]\n");
   while(TRUE)
   {
      printf(">> ");
      gets(input);
      if(sscanf(input,"%d",&year) && strlen(input) == 4 || !stricmp(input,"0"))
      {
         if(!strcmp(input,"0"))
            return;

         break;
      }

      printf("Please input a year(in numbers(4 digits)");
   }

   movie[0] = toupper(movie[0]);
   for(i = 1; i < strlen(movie); i++)
      movie[i] = tolower(movie[i]);

   strcpy(data[*id].movie,movie);
   data[*id].year = year;
   *id += 1;
   data[*id-1].id = *id;

} // End function data_Add

data_Del(int *id, database data[100])
{
   database temp[100];

   char input[10];

   int del;
   int i;
   int j = 0;

   if(!*id)
   {
      printf("You don't have any items in your database!\n\n");
      return;
   }

   printf("Please enter the ID of the item you want to delete(0 = exit)\n");
   printf("do not include a 0 infront of 1-9!\n");
   while(TRUE)
   {
      printf(">> ");
      gets(input);
      if(sscanf(input,"%d",&del) && del <= *id)
      {
         if(!stricmp(input,"0"))
            return;
         break;
      }
      printf("Please input a valid ID!!");
   }

   for(i = 0; i < *id; i++)
      temp[i] = data[i];

   for(i = 0; i < *id; i++)
   {
      if(i+1 == del)
         j += 1;

      strcpy(data[i].movie,temp[j].movie);
      data[i].year = temp[j].year;
      data[i].id = i+1;
      j += 1;
   }

   *id -= 1;

} // End function data_Del
Cubbi (4774)
The gaping hole I see is

1
2
3
 
   char input[10];
   [...]
         gets(input);


No program should have a call to gets() in it. It took the standards committees until recently to officially deprecate this function in C and C++, but it's been long known that it cannot be used safely, at all.

It lets any user get your program to execute arbitrary user-supplied code with your permissions (unless your system has non-executable stacks or some other defense from buffer overflow exploits)


kusen (3)
Thank you allot for the feedback. I'm assuming fgets is the correct function to use then? Edited my code to have that instead(however can't actually edit it from the code snippet on the page as it gets to long :\.
Topic archived. No new replies allowed.