CODEWITHSUNDEEP
c++functionloopsbooleanreturn-value
Cription
Cription

Reputation: 1

C++ | How Do I Loop an If Statement Without Returning a Value Each Time

I have a loop within a function that increments through an array checking each element to see if it matches the parameter of the function.

My problem is the requirements for returning a value. Since returning a value completes the function, if one wrong match returns false and I'm required to return a value at every control path, then I wont be able to check any of the other elements within the array to see if they match.

Most of my experience is in AS3, in which this function structure does work. How would I go about structuring this function properly in C++?

Function: It's purpose is to check if an inputted command exists and return a boolean if true or false

bool Commands(string _input)
{
    string cmds[] = { "login", "logout", "exit" };

    int i = 0;
    do {
        if (_input == cmds[i])
        {
            return true;
        }
        else
        {
            if (i == sizeof(cmds))
            {
                return false;
            }
        }
        i++;
    } while (i < sizeof(cmds));
}

Upvotes: 0

Views: 1582

Answers (3)

Vlad from Moscow
Vlad from Moscow

Reputation: 310920

For starters it is better to declare the parameter as constant reference. There is no need to declare an array of objects of the type std::string. It is much better to declare an array of string literals.

The loop is wrong. For example this expression sizeof(cmds) does not make sence in conditions like for example this

 if (i == sizeof(cmds))

The number of elements in the array is calculated like

sizeof(cmds) / sizeof( *cmds)

The function can be much simplified using standard algorithms.

Just write

#include <iostream>
#include <iomanip>
#include <string>
#include <algorithm>
#include <iterator>

bool Commands( const std::string &s )
{
    const char * cmds[] = { "login", "logout", "exit" };

    return std::find( std::begin( cmds ), std::end( cmds ), s ) != std::end( cmds );
}

int main() 
{
    std::cout << std::boolalpha << Commands( "logout" ) << std::endl;
    std::cout << std::boolalpha << Commands( "hello" ) << std::endl;

    return 0;
}

The program output is

true
false

If you want to use a loop in the function then it can look like it is shown below

#include <iostream>
#include <iomanip>
#include <string>

bool Commands( const std::string &s )
{
    const char * cmds[] = { "login", "logout", "exit" };
    const size_t N = sizeof( cmds ) / sizeof( *cmds );

    size_t i = 0;

    while ( i < N && s != cmds[i] ) i++;

    return i != N;
}

int main() 
{
    std::cout << std::boolalpha << Commands( "logout" ) << std::endl;
    std::cout << std::boolalpha << Commands( "hello" ) << std::endl;

    return 0;
}

It is not a good style of programming when a simple function has several exits and moreover in most cases it is not necessary.

Upvotes: 0

Jon Purdy
Jon Purdy

Reputation: 54971

I would suggest a few improvements:

sizeof(cmds) returns the size in bytes of cmds, which will be sizeof(string) * 3 or for example 96 on a 64-bit system; your loop will run off the end of the array, causing undefined behaviour. One option is to use sizeof(cmds)/sizeof(*cmds) to get the number of elements, but for your purposes it would be simpler to use a std::vector with an initializer list:

std::vector<string> cmds { "login", "logout", "exit" };

Next, you can replace your do loop with a for loop, and sizeof with the .size() member function of std::vector. If the input doesn’t match, you simply don’t return and proceed to the next iteration of the loop. If the function hasn’t returned after the loop, then no item was found, so you can return false.

for (size_t i = 0; i < cmds.size(); ++i)
  if (_input == cmds[i])
    return true;
return false;

You can also replace the loop with a call to the std::find algorithm from the standard <algorithm> header:

return std::find(std::begin(cmds), std::end(cmds), _input) != std::end(cmds);

Upvotes: 1

Ben H
Ben H

Reputation: 3236

While you are looping, return true when you find the value you are looking for. Otherwise, when the loop completes, you can simply return false after the loop.

bool Commands(string _input) {
    string cmds[] = { "login", "logout", "exit" };

    int i = 0;
    do {
        if (_input == cmds[i]) {
            return true; // we found it!
        }
        i++;
    } while (i < sizeof(cmds));
    return false; // we didn't find it.
}

Upvotes: 0

Related Questions