Primitive solution of squeeze

Question

As some of you might know I'm working my way through The C Programming Language as recommended by the "The Definitive C Book Guide and List" on StackOverflow.

I just finished my version of squeeze(s1, s2), a function to remove all occurrences of s2 in s1 and was hoping to get some feedback on my solutions.

This exercise is part of the second chapter (ex 2.4) so we haven't really gone through pointers and de-referencing yet. So basically all experience I have with the language is with using basic and primitive data types and the previous feedback I've received here!

All sort of feedback would be greatly appreciated; syntactical changes, algorithmic shortcomings etc. Just keep in mind that I just recently started out with the language!

---

Here is the code:

/*
    Exercise 2.4
    squeeze (s1, s2): Remove all characters of s2 in s1.
    INPUT : s1.length >= s2 > 0.
    OUTPUT: The rest of s1 after deleting all occurances of letters in s2.
*/

#include <stdio.h>

void squeeze (char s1[], const char *s2);  /* Returns (by-ref) the resulting string s1 after removing all occurences of s2. */
int toUpper(char c);                       /* Returns the numerical representation of a hexadecimal digit. */
int contains(char toCheck, const char *toRemove); /* Function to see if toRemove contains toCheck. */


int main () {
    char s1[] = "I am a test.\0";
    const char *s2 = "AE\0";
    printf("Before manipulation: %s\n", s1);
    squeeze(s1, s2);
    printf("After manipulation:  %s", s1);
}

/*
    Returns the (by-ref) resulting string s1 after removing all occurences
    of letters in s2.
*/

void squeeze (char s1[], const char *s2) {
    int index, left_shift;

    for (index = 0, left_shift = 0; s1[index] != '\0'; index++) {
        if (contains(s1[index], s2))
            continue;
        s1[left_shift] = s1[index];
        left_shift++;
    }
    s1[left_shift] = '\0';
}
/*
    Returns the upper-case representation of char c.
*/

int toUpper (char c) {
    if (c >= 'a' && c <= 'z')
        return c - 32;
    else
        return c;
}

/*
    Returns an indicator (integer) of if
    the string toRemove contains toCheck.
*/
int contains(char toCheck, const char *toRemove) {
    int index = 0, c = 0;

    while (toRemove[index] != '\0') {
        if ((c = toUpper(toCheck)) == toRemove[index]){
            return 1;
        }
        index++;
    }
    return 0;
}

Answer 1

The code is relatively neat and appears to work properly, so you seem to be well on your way to mastering C. Here are some things that may help you on that road.

Strings are automatically terminated

The code currently includes these two lines:

char s1[] = "I am a test.\0";
const char *s2 = "AE\0";

However, the explicit '\0' at the end of these strings is not necessary. A C string is automatically terminated with an implicit '\0' character, so these lines would normally be written as:

char s1[] = "I am a test.";
const char *s2 = "AE";

Move function bodies to avoid declaration

Modern C no longer requires function declarations before every function definition. As long as, reading from top to bottom, each function definition does not refer to anything not yet defined, the function declarations may be omitted. So in this program, if you put the function definitions in the order toUpper, contains, squeeze and main, you can omit the declarations. This is handy because it means you only need to maintain the function signature in one place rather than two.

Use bool instead of int where appropriate

The contains function returns 1 if the value is found or 0 otherwise. That is, it's essentially returning a boolean value. You can make that more clear by using #include <stdbool.h> and writing the function as:

bool contains(char toCheck, const char *toRemove) { /* ... */ }

Then the return statements can be return true; and return false; As documented here, bool has been standard C since 1999.

Use standard functions

I know this is a learning exercise and you're just starting, but it's generally encouraged to use standard functions rather than duplicating them. For instance, toupper has been part of the standard since 1989.

Avoid creating unneeded variables

The variable c in the contains() routine is not really needed. Instead of this:

if ((c = toUpper(toCheck)) == toRemove[index]){

You can write this:

if (toUpper(toCheck) == toRemove[index]){