if condition too long and including for loop

Question

Hi I am a C++ beginner and here is a problem I am facing when writing a function.

The bool function isData is used to see whether the array members are all numbers. Usually the size will be 11 or 9 or 7 so I don't want to hard code it. But I am not sure whether for loop work in if condition. And I will be so grateful if you can show me an easier way to do this.

bool isData(string temp[], int size)
{
  if(
      for (int i;i<size;i++) 
      {
        59 > +temp[i].at(0) && +temp[i].at(0) > 47 || +temp[i].at(0) == 45
      }
    )
    {
      return true;
    }
  else 
    {
      return false;
    }
}

You probably don't know `std::all_of` ? I assume you're working from a quite old C++ tutorial, since that `string[]` is about 20 years old. That predates even the 1998 variant of C++, while `std::all_of` is only 6 years old (2011) — MSalters, May 22 '17 at 14:59
You can't use a for loop as a condition of an if statement. The condition needs to be an expression and a for loop is not an expression. — Anon Mail, May 22 '17 at 15:06
Additionally, a test for `is_digit` is more legible than checking the ASCII values yourself. And checking for `== '-'` would be more legible than checking against 45. — AndyG, May 22 '17 at 15:08
Check this https://stackoverflow.com/questions/4654636/how-to-determine-if-a-string-is-a-number-with-c — Yan Khonski, May 22 '17 at 15:15
You could use a lambda in the if, but why not just make it a function? — Donnie, May 22 '17 at 16:11

Yan Khonski · Answer 1 · 2017-07-27T19:46:30.980

1

if (boolean_expression) You cannot put expression that returns nothing inside of if.

bool isData(string temp[], int size)
{
    for (int i = 0; i < size; i++) 
    {
        char* p;
        long converted = strtol(temp[i], &p, 10);
        if (*p) 
        {
            // conversion failed because the input wasn't a number
            return false;
        }
    }
    return true;
}

And check this: How to determine if a string is a number with C++?

For double you need to use strtod and instead of long use double.

edited Jul 27 '17 at 19:46

answered May 22 '17 at 15:16

Yan Khonski

12,225
15
76
114

Worse, a `for` statement is not an expression at all. – Lightness Races in Orbit May 22 '17 at 15:20
`if (*p)`? Did you mean `if (p)`? – Lightness Races in Orbit May 22 '17 at 15:38
@Jan Khonski will this handle negative and decimals? – Aravind Voggu May 22 '17 at 15:56
I added a link how to check if a number. **strtod** - it will return double https://stackoverflow.com/a/16575025/4587961 – Yan Khonski May 22 '17 at 16:00
@BoundaryImposition **if (*p)? Did you mean if (p)?** - No. if (p) will be true even when conversion is successful. – Yan Khonski May 22 '17 at 16:13
@JanKhonski: What are you trying to achieve with `if (*p)`? – Lightness Races in Orbit May 22 '17 at 16:19
if `temp` is either a number (and only a number), or empty, `p` will be pointing at `\0`. If `temp` is not a number, or is a number follwed by non-numerical characters, `p` will be pointing at said character. I think he probably wanted `!*p` since a 0 here indicates success. – Donnie May 22 '17 at 16:26
Although I'm not sure what the for loop is for, since he's running `strtol` on the exact same thing every time. – Donnie May 22 '17 at 16:28
Hi! Thank you for your answer! I checked the link and almost understand how it works. It is just when I put that into my codes and run it, there will be an error suggestion says that the "temp" is incompatible with parameter of type "const char"...so I am confused. – Liz May 22 '17 at 17:02

Aravind Voggu · Accepted Answer · 2017-05-22T16:28:07.030

0

for loops doesn't result in true or false, so for in an if won't work.

instead, run all elements in a for loop and return false if you find a non-number. Otherwise, return true.

Pseudo code for that would be something like this

bool isData(String temp[], int size) {
    for(int i=0; i < size; i++) {
        if( hasOnlyDigits(temp[i]) ) { return false; }
    }
    return true;
}

hasOnlyDigits is a function that would take in string and check if it's a valid number.

Here is a one liner for positive numbers. bool hasOnlyDigits = (temp[i].find_first_not_of( "0123456789" ) == string::npos);

It's non trivial to implement a function which handles all inputs like +.23, -33--33. Implement a function that can handle inputs within your input constraints.

I'll update this answer if I find a robust way to check.

edited May 22 '17 at 16:28

answered May 22 '17 at 15:18

Aravind Voggu

1,491
12
17

@JanKhonski Thanks :). There was a typo there. I edited it now. – Aravind Voggu May 22 '17 at 15:22
OK, but consider negative numbers as well. – Yan Khonski May 22 '17 at 15:22
1

what if there is a string --33-33. It will pass your test. Create a unit test for a method isNumber. – Yan Khonski May 22 '17 at 15:28
@JanKhonski I'm solving that and I'll be back with an edit. Thanks for pointing that out. – Aravind Voggu May 22 '17 at 15:38
Thank you! I have tried it and it works perfectly for now. And thanks for the detailed explanation:-) – Liz May 22 '17 at 16:56

score 0 · Answer 3 · answered May 22 '17 at 16:51

If you can compile with C++11 then you can use std::stoi. Similar to one of the answers above but with better error checking and ensuring loop runs over the whole array of strings.

bool isData(std::string temp[], int str_size)
{

    bool ret_val = false;
    size_t current_len, processed_len;
    for (int i=0; i < str_size; i++) {
        current_len = temp[i].size();
        try {
            int hidden_num = std::stoi(temp[i], &processed_len);
            std::cout << hidden_num << std::endl;
            if (processed_len == current_len) {
                ret_val = true;
                continue;
            }
        } catch (std::invalid_argument& e) {
            std::cout << "Non-integer input at index " << i << std::endl;
            ret_val = false;
            break;
        }
    }
    return ret_val;
}

score 0 · Answer 4 · answered May 22 '17 at 17:26

0

This is unnecessary:

if (...some complicated boolean expression...) {
    return true;
} else {
    return false;
}

All you need to do is:

return ...some complicated boolean expression...;

answered May 22 '17 at 17:26

Solomon Slow

25,130
5
37
57

if condition too long and including for loop

4 Answers4