LinuxQuestions.org - [SOLVED] Looking for feedback

- Programming (https://www.linuxquestions.org/questions/programming-9/)

- - Looking for feedback - C (https://www.linuxquestions.org/questions/programming-9/looking-for-feedback-c-4175724498/)

Looking for feedback - C

I've been writing this simple banking program for the past few weeks off and on. Posting for feedback, constructive criticism. Be gentle.

https://gitlab.com/jmgibson1981/basicbanker

the libraries i've built for this are here

https://gitlab.com/jmgibson1981/mylibraries

do not use hardcoded full path like this

Quote:

#include "/home/jason/Documents/Git/mylibraries/lib-jmgeneral.h"

please provide a makefile or a build script (or some description how to make it work)

What you've called string_return is known as strdup in unix. That's just FYI, I wouldn't suggest using strdup if you're writing portable C, but you might think about the naming differently (focusing on the "return" aspect of it looks a bit funny to me).

Code:

    deposit = prompt_scan_f("enter deposit value");

    // make sure valid dollar value

    if (string_valid_dollar(deposit)) {

      deposit = string_return(deposit);

Since prompt_scan_f is already allocating its result, the call to string_return here is redundant and causes a memory leak. And there is a similar problem in name_check.

Code:

  // open file for reading. if fail to open then create new file

  acctinfo = fopen(ACCTFILE, "r");

Why not just open it for appending (it will automatically create the file as needed)?

Code:

  // set up string to search for

  char * searchstr = (char*)malloc((strlen(acctnum) + 1) * sizeof(char));

  strcpy(searchstr, "acct");

  strcat(searchstr, acctnum);

The allocation is too small here (you allocated room for acctnum only but you're putting 4 additional characters), so you're probably going to have memory corruption.

Quote:

Why not just open it for appending (it will automatically create the file as needed)?

Because it does goes to a different function if the file needs to be created. First run it will create. After that it will always open to read. Unless there is a better way to do that.

I will continue working and exploring. I appreciate the response.

*EDIT*

Cleaned it up. Think i got the memory leaks, had more than I realized. The libraries are in the same repository as the program now so its in one place.

https://gitlab.com/jmgibson1981/basicbanker

Quote:

Originally Posted by jmgibson1981 (Post 6427403)

Because it does goes to a different function if the file needs to be created. First run it will create. After that it will always open to read. Unless there is a better way to do that.

Oh, I see, I misread the append_to_file function. It looks a bit convoluted though, do you really need to make a temp copy of your whole database just to find the next account number?

I don't have to do that for the account number now. I wrote a function to find the highest integer in a given column separated by delimiter. To calc the balance though with the file_search_func as it is I want it to get the first value that matches what it wants. As it writes from top to bottom it needs to start at the "bottom" to get the most recent.
I suppose I can stick the file flip in the file search function and leave it there. Is only place it gets used really.

*EDIT*

I suppose I could do the same as my highest_integer function and have it parse floats. Then I could drop the flip altogether. Keep the function but no need in this case...

It all seems to be working right now save one thing, oddly enough with the append_to_file function.

this is the output to the file. I just did the create new account over and over.

Code:

1,d,100.00,100.00,aslddjf aldfj

1,d,100.00,100.00,lasdjf asdokj

2,d,100.00,100.00,alkdjf alsdfj

3,d,100.00,100.00,alkdjf akldfj

3,d,100.00,100.00,lakdgjf klsgj

4,d,100.00,100.00,alsdfj aldkjf

4,d,100.00,100.00,alkdsjf aldj

5,d,100.00,100.00,alsdfj alkdjf

5,d,100.00,100.00,alkdsjf afldjh

6,d,100.00,100.00,alkdsjf akldj

Code:

void create_append(FILE * acctinfo, char * name, char * lname, char * deposit)

{

    int nextacct = highest_integer_column(acctinfo, ",", 0) + 1;

    // reopen file for appending

    acctinfo = fopen(ACCTFILE, "a");

    fprintf(acctinfo, "%d,d,%s,%s,%s %s\n", nextacct, deposit, deposit, name, lname);

    fclose(acctinfo);

}

Code:

int highest_integer_column(FILE * file, char * delimiter, int field)

{

  char buffer[256];

  char * token = NULL;

  int highnum = 0;

  int tokint = 0;

  int counter = 0;



  for (int i = 0; i <= file_num_lines(file); i++) {

    while (fgets(buffer, 256, file)) {

      token = strtok(buffer, delimiter);

      while(token) {

        if (counter == field) {

          tokint = atoi(token);

          if (tokint > highnum) {

            highnum = tokint;

          }

          counter = 0;

          break;

        } else {

          counter++;

          token = strtok(NULL, delimiter);

        }

      }

    }

  }

  rewind(file);

  return(highnum);

}

Solved it. New code.

Code:

void create_new_account(void) // working

{

  // declare & initialize + name prompts

  FILE * acctinfo = NULL;

  bool fileexist = false;

  char * name = name_check("enter first name");

  char * lname = name_check("enter last name");

  getchar();



  // initial deposit for new account

  char * deposit = NULL;

  while (deposit == NULL) {

    deposit = prompt_scan_f("enter deposit value", 24);

    // make sure valid dollar value

    if (!string_valid_dollar(deposit)) {

      free(deposit);

      deposit = NULL;

    }

  }



  // open file for reading. if fail to open then create new file

  if ((acctinfo = fopen(ACCTFILE, "r"))) {

    fileexist = true;

    fclose(acctinfo);

  }



  if (fileexist == true) {

    acctinfo = fopen(ACCTFILE, "r");

    int nextacct = highest_integer_column(acctinfo, ",", 0) + 1;

    fclose(acctinfo);

    acctinfo = fopen(ACCTFILE, "a");\

    fprintf(acctinfo, "%d,d,%s,%s,%s %s\n", nextacct, deposit, deposit, name, lname);

  } else {

    acctinfo = fopen(ACCTFILE, "w");

    fprintf(acctinfo, "1,d,%s,%s,%s %s\n", deposit, deposit, name, lname);

  }



  // clean up

  fclose(acctinfo);

  free(name);

  free(lname);

  free(deposit);

  name = NULL;

  lname = NULL;

  deposit = NULL;

  acctinfo = NULL;

}

Code:

int highest_integer_column(FILE * file, char * delimiter, int field)

{

  // declare & initialize

  char buffer[256];

  char * token = NULL;

  int highnum = 0;

  int tokint = 1;

  int counter = 0;



  // iterate through each line till EOF

  for (int i = 0; i <= file_num_lines(file); ++i) {

    while (fgets(buffer, 256, file)) {

      token = NULL;

      token = strtok(buffer, delimiter);

      while(token) {

        if (counter == field) {



          // if field = column to search then update tokint

          tokint = atoi(token);

          if (tokint > highnum) {

            highnum = tokint;

          }

          break;

        } else {

        // continue loop if not right field

          counter++;

          token = strtok(NULL, delimiter);

        }

      }

    }

    counter = 0;

  }



  rewind(file);

  return(highnum);

}