- object-oriented c++ functional-programming refactoring
- Updated Wed, 27 Jul 2022 03:27:17 GMT

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?

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.