CODEWITHSUNDEEP
cif-statementint
yeahright
yeahright

Reputation: 35

C Integer range must meet 3 different conditions

If I wanted to limit the range of values to be assigned to an integer to three different conditions. eg; Must be between 9 and 95 and also be divisible by 5 would this be the correct way to accomplish this?

I've been told that i can have multiple conditions as long as they are separated by && but I am having little success with my code. 

if (input >= 5 && input <= 95 && input %5)

Upvotes: 1

Views: 96

Answers (2)

paxdiablo
paxdiablo

Reputation: 881403

There are a number of issues with your code as it stands. First, the outright bugs:

  • The expression input % 5 will give you the remainder when divided by five. This means you will get zero if it is a multiple, non-zero otherwise. Unfortunately, zero is treated as false so this will only be true if input is not a multiple. The correct expression is (input % 5) == 0.

  • If you enter something that cannot be interpreted as an integer, the scanf will fail and input will be left at whatever value it was beforehand. This should be caught and acted upon, by checking the return value - this gives you the number of items successfully scanned so should be one.

  • Your code seems to return the value if okay but return nothing if it's invalid.

Next, while not bugs, these things are my personal preferences which can make code easier to read and maintain:

  • I prefer to explicitly separate sub-expressions so I never have to worry about precedence rules (provided it doesn't make the expression unreadable in the process). To that end, I would make the full if statement if ((input >= 5) && (input <= 95) && ((input % 5 == 0)).

  • I'm not a big fan of the if (condition) transferControl else ... construct since the else is totally superfluous.

  • I also prefer error catching to be done in a localised fashion at the start, catching problems early. Only after all checks are passed do you do the success portion.

  • A function (assuming it is a function, which seems likely) should generally do one thing, such as check if the value is valid. Writing issues to standard output is probably best left to the caller so that the function is truly re-usable. It would be better to have a function do the check and return some value to indicate whether or not there was a failure, along with the value if valid.

  • It's usually better to use puts("something") rather than printf("something\n"). The printf call is best left to where you actually need to do argument formatting.

Taking that all into account, the code that I would posit would be along the lines of:

#include <stdbool.h>

bool InputValidRangeAndMultiple(
    unsigned *pValue,
    unsigned minVal,
    unsigned maxVal,
    unsigned multVal
) {
    unsigned input;

    // If no unsigned int available, error.

    if (scanf("%u", pValue) != 1) return false;

    // If value invalid in any way (range or multiple), error.

    if ((*pValue < minVal) || (*pValue > maxVal)) return false;

    if ((*pValue % multVal) != 0) return false;

    // Value is now deemed okay.

    return true;
}

Calling that function can be done thus, with the prompts and errors handled outside the "input and check" function:

#include <stdio.h>

unsigned value;

puts("Enter Value.\nValue must be divisible by 5 and within 5 and 95...");
if (! InputValidRangeAndMultiple(&value, 5u, 95u, 5u)) {
    puts("Invalid input...");
    returnOrDoSomethingIntelligent();
}
// The 'value' variable is now valid.

Upvotes: 0

OrdoFlammae
OrdoFlammae

Reputation: 731

Your code seems fine to me, except for this line.

if (input >= 5 && input <= 95 && input %5)

The expression input % 5 returns the remainder of input/5. You want input to be divisible by 5, which happens when input % 5 returns a remainder of 0. Since C interprets 0 as false, and pretty much all other integers as true, this expression will do exactly the opposite of what you want it to do. Try using

if (input >= 5 && input <= 95 && (input % 5 == 0))

That should do what you want it to do.

Upvotes: 2

Related Questions