0

I cant get my second if statement to work like expected. It is adding data into the array even though it's validated as incorrect. For example: The console prompts;

Enter Score: 1 for student: 1

Then if 500 is entered, the following prompt appears:

Please enter a value between 0 and 100.

Enter Score: 2 for student 1.

It is not holding it at "Score 1" for correct data to be entered.

I don't understand why because the 1st if statement works like that, keeping the array at [0,0] until correct data is entered.

static bool IsInRange (int input)
{
    return input >= 0 && input <= 100;
}

for (int studentIndex = 0; studentIndex < studentCount; studentIndex++)
{
    for (int scoreIndex = 0; scoreIndex < scoreCount; scoreIndex++)
    {
        int parsedScore = -1;
        string score = string.Empty;
        while(!IsNumeric(score) && !IsInRange(parsedScore))
        {
            Console.WriteLine("Enter score: {0} for student: {1}", scoreIndex + 1, studentIndex + 1);
            score = Console.ReadLine();
            
            if (!IsNumeric(score))
            {
                Console.WriteLine(string.Empty);
                Console.WriteLine("Please enter a numeric value.");
                continue;
            }
            
            parsedScore = Convert.ToInt32(score);
            if (!IsInRange(parsedScore))
            {
                Console.WriteLine(string.Empty);
                Console.WriteLine("Please enter a value between 0 and 100");
            }
            
            studentScores[studentIndex, scoreIndex] = parsedScore;
            
        }
    }
}
Improve this question
2
  • 1
    It's easier to see if you separate the conditions: while(!IsNumeric(score) && !IsInRange(parsedScore)) IsNumeric(score) returns true when you enter 500, so the while loop doesn't continue executing Commented Mar 17, 2018 at 23:53
  • NB: Your code for studentScores[studentIndex, scoreIndex] = parsedScore; is inside your while loop. Not related to the issue itself, but is another problem (i.e. at that point the parsed score's invalid). Commented Mar 18, 2018 at 0:00

2 Answers 2

Reset to default
1

Your specific issue is the line !IsNumeric(score) && !IsInRange(parsedScore).

i.e.

Is Numeric | Is In Range | Result Of Condition
----------------------------------------------
True       | True        | False
True       | False       | False
False      | True        | False
False      | False       | True

... whilst you want

Is Numeric | Is In Range | Result Of Condition
----------------------------------------------
True       | True        | False
True       | False       | True
False      | True        | True
False      | False       | True

i.e. while (!IsNumeric(score) || !IsInRange(parsedScore)) or while (!(IsNumeric(score) && IsInRange(parsedScore)).

However, I'd recommend you refactor a bit further to this:

static bool IsInRange (int input)
{
    return input >= 0 && input <= 100;
}

static int GetScore (int studentIndex, int intscoreIndex)
{
    int parsedScore;
    var isValid = false;
    while (!isValid)
    {
        Console.WriteLine("Enter score: {0} for student: {1}", scoreIndex + 1, studentIndex + 1);
        var score = Console.ReadLine();
        if (IsNumeric(score))
        {
            parsedScore = Convert.ToInt32(score);
            if (IsInRange(parsedScore))
            {
                isValid = true;
            }
            else
            {
                Console.WriteLine(string.Empty);
                Console.WriteLine("Please enter a value between 0 and 100");
            }           
        }
        else
        {
            Console.WriteLine();
            Console.WriteLine("Please enter a numeric value.");
        }
    }
    return parsedScore;
}

.

for (int studentIndex = 0; studentIndex < studentCount; studentIndex++)
{
    for (int scoreIndex = 0; scoreIndex < scoreCount; scoreIndex++)
    {                
        studentScores[studentIndex, scoreIndex] = GetScore(studentIndex, scoreIndex);
    }
}

Additional Notes

  • I've moved your logic into GetScore; that helps keep the logic to get and validate the value seperate from where it's used / makes the code less complicated to read.
  • By having the IsValid flag you avoid assessing the same logic twice (i.e. once when actually validating, a second time when deciding whether to loop).
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

@CamiloTerevinto I tend to post code first, then go back and add explanation after. Most people care more about getting the working code than the explanation (so it gives them what they want before the boring wordy bit), and also gives them a chance to feed back before I put too much into the answer. My answers can be a lot more verbose (e.g. stackoverflow.com/questions/49338306/…)
That makes sense, but people don't always leave comments for downvotes nor take another look at the answer, so not sure if it's the best strategy. Reversed vote though
1

I think you have to add continue or it will just save it to the array. Since continue will force it to exit the current loop it will never save it to studentScores.

if (!IsInRange(parsedScore))
{
    Console.WriteLine(string.Empty);
    Console.WriteLine("Please enter a value between 0 and 100");
    continue;
}

While my first answer did not provide the solution JohnLBevan does explain the issue correctly and found the problem with the while loop.

while (!IsNumeric(score) || !IsInRange(parsedScore)) 
// or 
while (!(IsNumeric(score) && IsInRange(parsedScore)).
Improve this answer

2 Comments

While that's a problem in the current code, it's not the problem in the question
The downvote arrow says "this answer is not useful", and as it is, it's not. Edit your answer, add the fix to the actual problem (while also leaving this) and I'll upvote

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.