2
\$\begingroup\$

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 <iostream>
#include <string>

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 <= oneHandLength; ++a){

        // Left
        if(((input[a] - '0') > (input[a + 1] - '0')) && ((a + 1) < oneHandLength)){
            isValid = false; 
            break;
        }

        // Right
        if(((input[a + oneHandLength] - '0') < (input[(a + oneHandLength) + 1] - '0')) && (a + oneHandLength > 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;
}
\$\endgroup\$
  • \$\begingroup\$ "0111014321" is considered valid with your code. \$\endgroup\$ – Martin R Aug 28 '16 at 8:36
1
\$\begingroup\$

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.

  1. 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) ||
        ((input[a + oneHandLength] - '0') < (input[(a + oneHandLength) + 1] - '0') && a + oneHandLength > oneHandLength + 1));
    
  2. 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
    
  3. 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);
    
  4. 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)){
       ^^              ^   ^                  ^^    ^^     ^                ^
    
  5. You have inconsistent spacing, choose one and be consistent:

    for(int a = 0; a <= oneHandLength; ++a){
    int main () {
    if ((input[b] - '0')){
    if(isValid){
    
  6. 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.

\$\endgroup\$
  • \$\begingroup\$ Awesome! I noticed I tend to have an empty line after an if or for statement. Is it fine or should I have the opening bracket be on that empty line? \$\endgroup\$ – Poplop Aug 28 '16 at 15:52
  • 1
    \$\begingroup\$ @Poplop Doesn't matter, as long as you are consistent :) \$\endgroup\$ – Rakete1111 Aug 28 '16 at 15:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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