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;
            
        }
    }
}
Community
  • 1
  • 1
  • 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 – Camilo Terevinto Mar 17 '18 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). – JohnLBevan Mar 18 '18 at 00:00

2 Answers2

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)).
Camilo Terevinto
  • 31,141
  • 6
  • 88
  • 120
Matt
  • 135
  • 3
  • 17
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).
JohnLBevan
  • 22,735
  • 13
  • 96
  • 178
  • @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. https://stackoverflow.com/questions/49338306/sql-threadsafe-update-top-1-for-fifo-queue/49340199#49340199) – JohnLBevan Mar 18 '18 at 00:18
  • 1
    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 – Camilo Terevinto Mar 18 '18 at 00:22