FULL DISCLOSURE: SCHOOL ASSIGNMENT
I've been working on some code to pull data from a CSV and move it to another CSV, however I keep encountering this error I can't seem to overcome.
For the part I'm working on, the user supplies a command line argument with 'DELETE OPT1' where OPT1 is the ID of an entry in the CSV. The deleteStuff() function should go through the database.csv and delete the first entry/row of a matching ID. It should achieve this by creating a database.tmp, then copying the database.csv over to the tmp, excluding the first matching entry. The csv is then deleted and then the tmp is renamed to database.csv, as if nothing happened.
However, the source code (database.csv) seems to be doing something wrong and deleting everything but the ID's. Below, I've posted the source code and the starting database.csv, as well as what the code outputs after running DELETE 10. Any help would be appreciated, especially in understanding how to the next line of a fgets().
Note that the noSpaces() function just removes any empty spaces since it is possible for them to be included in the input according to our prof.
database.c:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
char show[] = "SHOW";
char delete[] = "DELETE";
char add[] = "ADD";
void noSpaces(char* s) {
const char* d = s;
do {
while (*d == ' ') {
++d;
}
} while (*s++ = *d++);
}
void showStuff() {
FILE* csv = fopen("database.csv", "rt");
if (csv == NULL) {
printf("\n File opening failed");
exit(1);
}
char buffer[800];
char *Gptr, *ID, *name, *cAge, *cGPA;
int counter = 1;
while (fgets(buffer, sizeof(buffer), csv)) {
ID = strtok(buffer, ",");
noSpaces(ID);
name = (strtok(NULL, ","));
noSpaces(name);
cAge = (strtok(NULL, ","));
noSpaces(cAge);
int age = atoi(cAge);
cGPA = (strtok(NULL, ","));
noSpaces(cGPA);
double GPA = strtod(cGPA, &Gptr);
printf("Record %d: ID=%-5s NAME:%-5s AGE:%-5d GPA:%.1f\n", counter, ID, name, age, GPA);
counter++;
}
fclose(csv);
}
void deleteStuff(char givenID[]) {
FILE* csvread = fopen("database.csv", "rt");
if (csvread == NULL) {
printf("\n File opening failed");
exit(1);
}
FILE* csvwrite = fopen("database.tmp", "wt");
char buffer[800];
char* ID;
int oneAndDone = 0;
while (fgets(buffer,sizeof(buffer),csvread)) {
ID = strtok(buffer, ",");
noSpaces(ID);
if ((strcmp(ID, givenID) == 0) && (oneAndDone == 0)) {
oneAndDone++;
continue;
}
fprintf(csvwrite, "%s", buffer);
}
system("rm database.csv");
system("mv database.tmp database.csv");
fclose(csvread);
fclose(csvwrite);
if (oneAndDone == 0) {
printf("Sorry, the user was not found. Nothing was deleted.\n\n");
exit(1);
}
}
void addStuff(char gID[], char gName[], char gAge[], char gGPA[]) {
char* Gptr;
int age = atoi(gAge);
double GPA = strtod(gGPA, &Gptr);
FILE* csvappend = fopen("database.csv", "at");
if (csvappend == NULL) {
printf("\n File opening failed");
exit(1);
}
fprintf(csvappend, "%s,%s,%d,%.1f", gID, gName, age, GPA);
fclose(csvappend);
}
void main(int argc, char* argv[]) {
if (argc == 1) {
printf("Your did not provide any arguments. Please enter: ./database CMD OPT1 OPT2 OPT3 OPT4 \n\n");
exit(1);
}
if (strcmp(argv[1], show) == 0) showStuff();
else if (strcmp(argv[1], delete) == 0) {
if (argc <= 2) {
printf("Name of record to delete is missing\n\n");
exit(1);
}
deleteStuff(argv[2]);
}
else if (strcmp(argv[1], add) == 0) {
if (argc <= 5) {
printf("Missing ID, Name, AGE, and GPA Arguments\n\n");
exit(1);
}
addStuff(argv[2], argv[3], argv[4], argv[5]);
}
else printf("The command you requested in invalid. Please select from one of these: SHOW, DELETE, ADD\n\n");
}
Original database.csv:
10,bob,18, 3.5
15,mary,20,4.0
5,tom, 17, 3.8
After database.csv:
155
When using strtok
you must understand it modifies the string that you are tokenizing. If you will need to use the string after calling strtok
, you should make a copy of the string first. Always check the man page
for any function you are using if you are unsure exactly how it works, e.g. man 3 strtok
Be cautious when using these functions. If you do use them, note that: * These functions modify their first argument.
Unless you are programming for a microcontroller of some other device without an operating system (a "freestanding" system) the use of void main()
is wrong. See: C11 Standard - §5.1.2.2.1 Program startup(p1). See also: What should main() return in C and C++?
You make use of the flag int oneAndDone = 0;
-- which is fine. However, after you have found the ID
provided on the command line and incremented oneAndDone++;
(or just set oneAndDone = 1;
), there is no longer a need to call strtok
at all. Wouldn't it make more sense to completely skip the strtok
call after oneAndDone
is no longer 0
? Something like:
char buffer[800];
char* ID;
int oneAndDone = 0;
while (fgets(buffer,sizeof(buffer),csvread)) {
if (oneAndDone == 0) {
char bcopy[800];
strcpy (bcopy, buffer);
ID = strtok(bcopy, ",");
noSpaces(ID);
if (strcmp(ID, givenID) == 0) {
oneAndDone = 1;
continue;
}
}
fprintf(csvwrite, "%s", buffer);
}
You have done a good job Not Skimping On Buffer Size with char buffer[800];
-- but don't use Magic-Numbers in your code either. Better:
...
#include <string.h>
#define MAXC 800 /* if you need a constant, #define one (or more) */
...
char buffer[MAXC];
char* ID;
int oneAndDone = 0;
while (fgets(buffer,sizeof(buffer),csvread)) {
if (oneAndDone == 0) {
char bcopy[MAXC];
...
You certainly want to close the files you are reading and writing from before you make system
calls to rm
and mv
. For example:
fclose(csvread);
fclose(csvwrite);
system("rm database.csv");
system("mv database.tmp database.csv");
You always want to check the return fclose
after-write to catch any stream error or problem with the file that may not be caught by checking only the write itself, e.g.
if (fclose(csvwrite) == EOF)
perror ("fclose-csvwrite");
That way you at least have an indication on whether the stream was flushed successfully and there were no errors on close.
Lastly, Enable Compiler Warnings and do not accept code until it compiles without warning. For gcc/clang use as minimum -Wall -Wextra -pedantic
(consider adding -Wshadow
as well). For VS use /W3
. For any other compiler, check the documentation and enable comparable warnings. That would point you to add enclosing parenthesis around:
} while ((*s++ = *d++));
in void noSpaces(char* s)
.
That should get you going. Think through and make the changes and let me know if you have further problem.