3
\$\begingroup\$

This program will perform a find and replace on a single word in a file, with the condition that the new word must have the same length as the old one. It will also display the number of occurrences of the old word.

I'm not interested in anything type of feedback, anything would be appreciated, it's a school assignment and it'll be reviewed by my teacher.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define WORD_SIZE 24

void write_to_file(const char *s);
void replace_word(const char *s, const char *old_word, const char *new_word);

int main(void){
    const char file_name[] = "text.txt";
    int c;
    char old_word[WORD_SIZE];
    char new_word[WORD_SIZE];
    puts("Enter 'w' to write to the file, 'r' to replace a word");
    while((c = getchar()) != EOF){
        switch(c){
        case 'w':
            getchar();
            write_to_file(file_name);
            break;
        case 'r':
            getchar();
            puts("Enter the word you want to replace");
            fgets(old_word, sizeof(old_word), stdin);
            old_word[strlen(old_word) - 1] = '\0';
            puts("Enter the new word");
            fgets(new_word, sizeof(new_word), stdin);
            new_word[strlen(new_word) - 1]  = '\0';

            if(strlen(old_word) != strlen(new_word)){
                printf("Error: can't replace \"%s\" with \"%s\":\n\"%s\" the length is not the same", old_word, new_word, new_word);
                break;
            }
            replace_word(file_name, old_word, new_word);
            break;
        default:
            printf("Invalid command:%i\n", c);
            break;
        }
    }
    return 0;
}

void write_to_file(const char *s){
    FILE *in_file;
    if((in_file = fopen(s, "w")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }
    int c;
    while((c = getchar()) != EOF){
        fputc(c, in_file);
    }
    fclose(in_file);
}

unsigned long fsize(const char *s){
    FILE *f;
    if((f = fopen(s, "r")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }
    fseek(f, 0, SEEK_END);
    unsigned long len = ftell(f);
    fclose(f);
    return len;
}

void replace_word(const char *s, const char *old_word, const char *new_word){
    FILE *original_file;
    FILE *copy;

    if((original_file = fopen(s, "r")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }

    if((copy = fopen("copy.txt", "w")) == NULL){
        perror("text");
        exit(EXIT_FAILURE);
    }

    const int BUFFER_SIZE = fsize(s);
    char *buffer = malloc(BUFFER_SIZE);
    char *init_loc = buffer;

    int word_len = strlen(old_word);
    int word_frequency = 0;

    while(fgets(buffer, BUFFER_SIZE, original_file)){
        while((buffer = strstr(buffer, old_word))){
            memcpy(buffer, new_word, word_len);
            word_frequency++;
        }
        buffer = init_loc;
        fprintf(copy, "%s", buffer);
    }
    printf("'%s' found %i times\n", old_word, word_frequency);

    fclose(original_file);
    fclose(copy);

    if((original_file = fopen(s, "w")) == NULL){
        perror(s);
        exit(EXIT_FAILURE);
    }

    if((copy = fopen("copy.txt", "r")) == NULL){
        perror("text");
        exit(EXIT_FAILURE);
    }

    int c;
    while((c = fgetc(copy)) != EOF){
        fputc(c, original_file);
    }

    fclose(original_file);
    fclose(copy);
    free(buffer);
}
\$\endgroup\$
  • \$\begingroup\$ sed -e 's/<word1>/<word2>/g' \$\endgroup\$ – Martin York May 16 '17 at 0:14
  • \$\begingroup\$ @LokiAstari "School assignment" \$\endgroup\$ – user127566 May 16 '17 at 9:48
1
\$\begingroup\$

Small suggestions:

  • Be consistent. What jumps out right away is that you say "This program will perform a find and replace on a single word in a file" but then your program prompts the user to either write to file or replace a word. That confuses me as a user of your program as to what it's supposed to do.

  • Removed. Refer to Difference between int and char in getchar/fgetc and putchar/fputc?

    You've declared int c in main(). Why int and not char ? This also comes up when your case statement reaches the default part. When user enters wrong command, say g, it displays:

    Enter 'w' to write to the file, 'r' to replace a word
    g
    Invalid command:103
    Invalid command:10
    

    So avoid confusing the user and the people who are going to read your code.

  • In fgets(old_word, sizeof(old_word), stdin); why do you use sizeof(old_word) ? You already have a constant for WORD_SIZE defined, so there's no need to figure out size of your string.

  • Consider moving the code from case 'r' for getting user's input to replace_word() function or separate function get_user_input(). What was nice about your case statement is that you started out with case 'w' where you executed a specific function for that case. That's good strategy. Use that case statement to direct the program to specific functions that do specific job.
  • When user selects to replace a word, you give the user instructions to enter old and new string. That's good - user is not left hanging and is guided in what to do. But when user selects to write to file, there's no instruction and user is just dropped into typing stuff.

    I only clicked Ctrl+D ( to send EOF character since I'm Linux user, not sure how would that work on Windows) because I know that this is what program expects to receive to indicate that there's no more input, but what about a random user who's absolutely unfamiliar with your program and never read your source code ?

  • On the same related note, how does user exit your program ? In your case statement you only have two cases for specific action, there's no quit option, and neither does it explain to the user that they could use Ctrl+C or Ctrl+D to quit.
  • Give the user Enter 'w' to write to the file, 'r' to replace a word each time an action has been complete. Take for example the example I showed in my first point. User enters wrong command, they see an error and . . . what's next ? The program just sits there waiting for input without explaining to the user what it wants.
\$\endgroup\$
  • \$\begingroup\$ Your suggestion of char c in main will lead to bugs. Also, using sizeof old_word is way better than using a named constant. \$\endgroup\$ – Roland Illig May 15 '17 at 18:43
  • \$\begingroup\$ @RolandIlligs for instance ? \$\endgroup\$ – Sergiy Kolodyazhnyy May 15 '17 at 18:44
  • \$\begingroup\$ Just search for the keywords getchar int char. \$\endgroup\$ – Roland Illig May 15 '17 at 18:51
  • \$\begingroup\$ @RolandIllig OK, I did find this. Wasn't aware of such behavior for signed and unsigned chars. On side note, your comment wasn't really helpful. Would be much nicer if you actually pointed me to a specific link instead of saying "go search for it". \$\endgroup\$ – Sergiy Kolodyazhnyy May 15 '17 at 18:57
  • \$\begingroup\$ @RolandIllig removed that part , by the way \$\endgroup\$ – Sergiy Kolodyazhnyy May 15 '17 at 18:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy