Is there a better way to code the function that checks for an isogram?

[Ethan_See]

I wrote this code myself, and it does work, but is there a better way to write this? For example, other ways of returning values so I can avoid the if and else statements. Or ways of making the code cleaner and not so messy looking. Also, it is advisable to use intergers 1 and 0 to represent true and false or is it better to just use the boolean values?

My code is:

int32 UBullCowCartridge::IsIsogram(FString Input) const{ 
    int check = 0;        

    for (int o = 0;o < Input.Len()-1;++o) {
        for (int i = o+1;i < Input.Len();++i) {
            if (Input[o] == Input[i]) {
                return check = 1;
            }
        }   // End of for nested for loop
        
        
        
        if (check == 1) {
            return check;
        }
    } // End of outer for loop
    if (check == 1) {
        return check;
    }
    else
        return check;
}

The code uses nested for loops to iterate through a word and check if there are more than one of any character in the word and once it finds a character that has been repeated, it will change the value of check and return it.

Then, the if and else statements are so the function knows what value it should be returning, as I am using 1 to represent true and 0 to represent false. Since I cannot return a value straight from the nested loops as that causes the error, “not all control paths return a value”.

[DanM]

This is essentially what the course does.

@Ethan_See

return check = 1;

1. This is returning true if a letter was found in another position but it should be returning false.
2. Assigning a local variable and then returning it is a little strange. As you can just return the value e.g.

   return 1;
   

With that said

@Ethan_See

it is advisable to use intergers 1 and 0 to represent true and false or is it better to just use the boolean values?

Use true and false. That is why they (along with bool) were added to the C++ language.

C doesn’t have those but are available through a macro. I believe C actually adopted those macros because of the value it added to C++. So you should definitely use them.

@Ethan_See

    if (check == 1) {
        return check;
    }
    else
        return check;

This reads “if check == true, return check, otherwise …also return check”

@Ethan_See

Since I cannot return a value straight from the nested loops as that causes the error, “not all control paths return a value”.

Could you show what you originally had?

[Ethan_See]

Yes the code I sent was a little strange, I cleaned it up a little more so I am using booleans now, for the error that I got, originally the code did not have the if/ else statements where they return values. So the code would just be the same as above, just without the if/else that returned check. As a result, my overall function was not returning anything and as the function is bool, it must return true or false.

My code looks like this now after some changes:

bool UBullCowCartridge::IsIsogram(FString Input) const{ 
    bool check = true;

    for (int o = 0;o < Input.Len()-1;++o) {
        for (int i = o+1;i < Input.Len();++i) {
            if (Input[o] == Input[i]) {
                return check = false;
            }
        }   // End of for nested for loop
    } // End of outer for loop, don't need to use return in the outer loop, return makes it to function IsIsogram scope
    return check;
}

I think it looks better now, as I used a notebook to write my thought process down, which helped simplify things.

[DanM]

And now hopefully you can see now that the bool variable isn’t needed at all. You know the exact values at compile time that you wish to return at both points.

Which would make your code exactly the same as what Mike came up with (though he forgot to do the -1). So congrats