4

I have implemented the cows and bulls game in C++. The code:

#include <cstdio>
#include <cstdlib>
#include <ctime>


struct DigitMatches {
    int matches_in_right_positions;
    int matches_in_wrong_positions;
};

struct MatchesCounterData {
    int generated_number_left_digits[10];
    int guess_number_left_digits[10];
};

void CountNumberDigits(int (&counts)[10], int n) {
    while (n > 0) {
        counts[n % 10]++;
        n /= 10;
    }
}

int CountMatchesInRightPositions(MatchesCounterData *data, int generated_number, int guess_number) {
    int matches = 0;

    while (generated_number > 0 && guess_number > 0) {
        const int generated_number_digit = generated_number % 10;
        const int guess_number_digit = guess_number % 10;

        if (generated_number_digit == guess_number_digit) {
            matches++;

            data->guess_number_left_digits[generated_number_digit]--;
            data->generated_number_left_digits[generated_number_digit]--;
        }

        generated_number /= 10;
        guess_number /= 10;
    }

    return matches;
}

int CountMatchesInWrongPositions(MatchesCounterData *data, int guess_number) {
    int matches = 0;

    while (guess_number > 0) {
        const int guess_number_digit = guess_number % 10;

        int &generated_number_left_digits = data->generated_number_left_digits[guess_number_digit];
        int &guess_number_left_digits = data->guess_number_left_digits[guess_number_digit];

        if (generated_number_left_digits > 0 && guess_number_left_digits > 0) {
            matches++;

            generated_number_left_digits--;
            guess_number_left_digits--;
        }

        guess_number /= 10;
    }

    return matches;
}

DigitMatches CheckMatchedDigits(int generated_number, int guess_number) {
    MatchesCounterData matches_counter_data = {};

    CountNumberDigits(matches_counter_data.generated_number_left_digits, generated_number);
    CountNumberDigits(matches_counter_data.guess_number_left_digits, guess_number);

    DigitMatches matches;

    matches.matches_in_right_positions = CountMatchesInRightPositions(&matches_counter_data,
                                                                      generated_number, guess_number);
    matches.matches_in_wrong_positions = CountMatchesInWrongPositions(&matches_counter_data, guess_number);

    return matches;
}

void InitializeRandomNumberGenerator() {
    srand(time(nullptr));
}

int GenerateNumber(int min, int max) {
    return min + (rand() % (max - min + 1));
}

int InputNumber() {
    int n;
    scanf("%d", &n);
    return n;
}

int main() {
    InitializeRandomNumberGenerator();

    const int kTotalNumberOfDigits = 4;
    const int generated_number = GenerateNumber(1000, 9999);

    int attempts = 0;

    while (true) {
        const int guess_number = InputNumber();

        if (guess_number < 1000 || guess_number > 9999) {
            printf("Invalid number. Must be between 1000 and 9999.\n");
            continue;
        }

        const DigitMatches matches = CheckMatchedDigits(generated_number, guess_number);

        printf("%d cows, %d bulls\n", matches.matches_in_right_positions, matches.matches_in_wrong_positions);

        attempts++;

        if (matches.matches_in_right_positions == kTotalNumberOfDigits)
            break;
    }

    printf("You won! Attempts: %d\n", attempts);

    return 0;
}

The question. Should I use classes instead of functions with a state needed for computation? In my case, the functions are CountMatchesInRightPositions and CountMatchesInWrongPositions, the state is MatchesCounterData structure. It seems like it's reasonable to create a class for this functions, because a state is mostly about classes and because these functions should be private I think, because they must be called in a specific order and MatchesCounterData values must be pre-computed before their call. In view of the above, those functions should not be public due to the requirements for calling them. I think they should be encapsulated in another function that would follow the requirements. Currently, that function is CheckMatchedDigits. So, another implementation of that code but using a class:

#include <cstdio>
#include <cstdlib>
#include <ctime>


struct DigitMatches {
    int matches_in_right_positions;
    int matches_in_wrong_positions;
};

void CountNumberDigits(int (&counts)[10], int n) {
    while (n > 0) {
        counts[n % 10]++;
        n /= 10;
    }
}

class DigitsMatchChecker {
public:
    DigitMatches CheckMatchedDigits(int generated_number, int guess_number) {
        CountNumberDigits(this->generated_number_left_digits_, generated_number);
        CountNumberDigits(this->guess_number_left_digits_, guess_number);

        DigitMatches matches;

        matches.matches_in_right_positions = this->CountMatchesInRightPositions(generated_number, guess_number);
        matches.matches_in_wrong_positions = this->CountMatchesInWrongPositions(guess_number);

        return matches;
    }

private:
    int CountMatchesInRightPositions(int generated_number, int guess_number) {
        int matches = 0;

        while (generated_number > 0 && guess_number > 0) {
            const int generated_number_digit = generated_number % 10;
            const int guess_number_digit = guess_number % 10;

            if (generated_number_digit == guess_number_digit) {
                matches++;

                this->guess_number_left_digits_[generated_number_digit]--;
                this->generated_number_left_digits_[generated_number_digit]--;
            }

            generated_number /= 10;
            guess_number /= 10;
        }

        return matches;
    }

    int CountMatchesInWrongPositions(int guess_number) {
        int matches = 0;

        while (guess_number > 0) {
            const int guess_number_digit = guess_number % 10;

            int &generated_number_left_digits = this->generated_number_left_digits_[guess_number_digit];
            int &guess_number_left_digits = this->guess_number_left_digits_[guess_number_digit];

            if (generated_number_left_digits > 0 && guess_number_left_digits > 0) {
                matches++;

                generated_number_left_digits--;
                guess_number_left_digits--;
            }

            guess_number /= 10;
        }

        return matches;
    }

private:
    int generated_number_left_digits_[10] = {};
    int guess_number_left_digits_[10] = {};
};

void InitializeRandomNumberGenerator() {
    srand(time(nullptr));
}

int GenerateNumber(int min, int max) {
    return min + (rand() % (max - min + 1));
}

int InputNumber() {
    int n;
    scanf("%d", &n);
    return n;
}

int main() {
    InitializeRandomNumberGenerator();

    const int kTotalNumberOfDigits = 4;
    const int generated_number = GenerateNumber(1000, 9999);

    int attempts = 0;

    while (true) {
        const int guess_number = InputNumber();

        if (guess_number < 1000 || guess_number > 9999) {
            printf("Invalid number. Must be between 1000 and 9999.\n");
            continue;
        }

        DigitsMatchChecker digits_match_checker;
        const DigitMatches matches = digits_match_checker.CheckMatchedDigits(generated_number, guess_number);

        printf("%d cows, %d bulls\n", matches.matches_in_right_positions, matches.matches_in_wrong_positions);

        attempts++;

        if (matches.matches_in_right_positions == kTotalNumberOfDigits)
            break;
    }

    printf("You won! Attempts: %d\n", attempts);

    return 0;
}

On the one hand, now it looks better, because those functions are encapsulated in a class as well as the state now looks better as fields of the class. But on the other hand, the class is one-time usable. I mean that for another computation I need to create another instance of the class, because the state of the previous instance is not at beginning. Of course, I can create a method something like Reset that would reset the state to the beginning, but is it a good practice? Also, is it a good practice to have a class for one computation at all? Does this type of classes have a name?

3
  • 2
    @DocBrown, you will laugh, I'm just from there. codereview.stackexchange.com/questions/270666/…. Another user recommended me to ask this question here.
    – sshd
    Dec 4 '21 at 15:11
  • @DocBrown, I did it. Here I am asking about choosing between functional programming and object-oriented programming. On CodeReview I am asking about improvements on my code.
    – sshd
    Dec 4 '21 at 20:33
  • I would use a base class that has all the states as virtual with a default action, like a log state. Then write each implemented state as a new class derived from it. We do all of outs this way. Might have come from Dr. Dobbs a while back. Your's is ok, but not a great C++ implementation if that's what you're striving for,
    – Larry_C
    Dec 6 '21 at 20:16
3

There's nothing wrong with creating an instance of a class to be used for a short computation and then be thrown away. Long lived "entity" objects are not the only kind, there can also be ephemeral and temporary objects.

I can't say anything about your code (tl;dr) and it would probably be better to ask that part of the question on the code review site.

1

Here I am asking about choosing between functional programming and object-oriented programming.

You say you are asking about functional vs object-oriented, but neither of your solutions are functional. This is more OO vs imperative.

In this case, the OO solution is obviously better because it hides the data and ensures that they are only manipulated in appropriate ways.

But as for your specific question above, everything you can do with a functional approach you can also do using an Object-Oriented approach. In my opinion, mixing the two approaches in the same program can be a problem. If you mix the approaches, you will end up adding complexity to the code because you will have to include extra code to convert from one paradigm to the other.

Another thing to keep in mind is how well does the language support the style? Functional programming makes heavy use of higher order functions and higher kinded types to implement abstractions. Object Oriented programming makes heavy use of interfaces (classes containing pure-virtual functions.) C++ has strong support for the latter and little support for the former. This strongly suggests one approach over the other...

Not the answer you're looking for? Browse other questions tagged or ask your own question.