patterncppMinor
DailyProgrammer 280 Easy: Counting on Fingers
Viewed 0 times
countingdailyprogrammereasyfingers280
Problem
We are trying to count to 100 on two hands!
Input is given like so: LP LR LM LI LT RT RI RM RR RP (i.e. 0111011100)
Where:
Our job is to make sure the code is valid according to this counting scheme, and then show what the number represented is.
Original post can be found here.
My solution validates the input by checking to see if there is a "1" followed by a "0" for the "left" hand, and vice-versa for the right. I have it set up so that positions 3,4, and 5 aren't checked (isn't necessary as they can't invalidate the input).
Looking for feedback on best practice, convention, better ways to solve problem, and really just anything :)
EDIT: I am not interested in checking for incorrect input such as "0123456789"
Input is given like so: LP LR LM LI LT RT RI RM RR RP (i.e. 0111011100)
Where:
L = Left, R = Right, P = Pinky, R = Ring Finger, M = Middle Finger, I = Index Finger, T = ThumbOur job is to make sure the code is valid according to this counting scheme, and then show what the number represented is.
Original post can be found here.
My solution validates the input by checking to see if there is a "1" followed by a "0" for the "left" hand, and vice-versa for the right. I have it set up so that positions 3,4, and 5 aren't checked (isn't necessary as they can't invalidate the input).
Looking for feedback on best practice, convention, better ways to solve problem, and really just anything :)
EDIT: I am not interested in checking for incorrect input such as "0123456789"
#include
#include
int main () {
bool isValid = true;
int total = 0;
std::string input = "0111011100"; // This one is 37
// Divide by 2 because there is two hands; subtract 1 to zero index
int oneHandLength = (input.length() / 2) - 1;
// Verify
for(int a = 0; a (input[a + 1] - '0')) && ((a + 1) oneHandLength + 1)){
isValid = false;
break;
}
}
// Count up the total
if(isValid){
for(int b = 0; b < input.length(); b++){
if ((input[b] - '0')){
if(b < oneHandLength) total += 10;
else if(b == oneHandLength) total += 50;
else if(b == oneHandLength + 1) total += 5;
else total += 1;
}
}
std::cout << input << " is valid: " << total;
}
else std::cout << input << " isn't valid!";
return 0;
}Solution
Overall, your code is really good :) Good job.
The thing that bothers me is the inconsistent weird indentation, but that's probably because of copying and pasting.
-
The 2 verifying conditions do the same thing, you can put them together, which reduces maintenance in the long term, although for such a small project it's not really important.
-
Depending on the architecture,
-
Don't use integers as booleans. It is better to clearly state that you want to check if the integer is greater than
-
Although using parenthesis reduces confusion, I think you are using too many of them:
-
You have inconsistent spacing, choose one and be consistent:
-
You aren't including a newline after the last output, which can be a "problem" when run in a shell:
Minor thing: You don't need to
The thing that bothers me is the inconsistent weird indentation, but that's probably because of copying and pasting.
-
The 2 verifying conditions do the same thing, you can put them together, which reduces maintenance in the long term, although for such a small project it's not really important.
if (((input[a] - '0') > (input[a + 1] - '0') && (a + 1) oneHandLength + 1));-
Depending on the architecture,
int is not guaranteed to cover the entire range of std::size_t, so you should use that as a type instead of int to avoid overflowing. This is not a problem now because you only have 10 fingers, but if you decide to add more... :)std::size_t oneHandLength = (input.length() / 2) - 1;
//'auto' would also be possible if you want to-
Don't use integers as booleans. It is better to clearly state that you want to check if the integer is greater than
0 (or equal to 1 in this case):if (input[b] - '0' == 1);-
Although using parenthesis reduces confusion, I think you are using too many of them:
if ((input[b] - '0')){
^ ^
if(((input[a] - '0') > (input[a + 1] - '0')) && ((a + 1) < oneHandLength)){
^^ ^ ^ ^^ ^^ ^ ^-
You have inconsistent spacing, choose one and be consistent:
for(int a = 0; a <= oneHandLength; ++a){
int main () {
if ((input[b] - '0')){
if(isValid){-
You aren't including a newline after the last output, which can be a "problem" when run in a shell:
$ ./hands
$ 0111011100 is valid: 37 $ (command here - on same line!)Minor thing: You don't need to
return 0;, so you can remove it if you want to.Code Snippets
if (((input[a] - '0') > (input[a + 1] - '0') && (a + 1) < oneHandLength) ||
((input[a + oneHandLength] - '0') < (input[(a + oneHandLength) + 1] - '0') && a + oneHandLength > oneHandLength + 1));std::size_t oneHandLength = (input.length() / 2) - 1;
//'auto' would also be possible if you want toif (input[b] - '0' == 1);if ((input[b] - '0')){
^ ^
if(((input[a] - '0') > (input[a + 1] - '0')) && ((a + 1) < oneHandLength)){
^^ ^ ^ ^^ ^^ ^ ^for(int a = 0; a <= oneHandLength; ++a){
int main () {
if ((input[b] - '0')){
if(isValid){Context
StackExchange Code Review Q#139820, answer score: 2
Revisions (0)
No revisions yet.