HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

DailyProgrammer 280 Easy: Counting on Fingers

Submitted by: @import:stackexchange-codereview··
0
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:

L = Left, R = Right, P = Pinky, R = Ring Finger, M = Middle Finger, I = Index Finger, T = Thumb

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"

#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.

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 to
if (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.