1

I'm trying to repeatedly get 2 inputs and store them into an array until the word 'end' is typed, afterwhich, I want to do is to give a 'grade' to each of the marks that was entered. However no matter what marks was entered, i was getting 'HD' for all the grades.

Any advice is appreciated! Thanks!

const readline = require('readline-sync');

var name, marks;
var studentList = [];

Input();

function printList(list) {
  for (let i = 0; i < studentList.length; i += 1) {
    var grade;
      if ((marks <= 100) && (marks => 80)){
          grade = 'HD'
          studentList[i][2] = grade;  

        } 
        else if ((marks <= 79) && (marks => 70)) 
        {
            grade = 'D'
            studentList[i][2] = grade;

        } 
        else if ((marks <= 69) && (marks =>60)) 
        {
            grade = 'C'
            studentList[i][2] = grade;

        } 
        else if ((marks <= 59) && (marks =>51)) 
        {
            grade = 'P'
            studentList[i][2] = grade;

        } 
        else if ((marks < 50) && (marks =>0)) 
        {
            grade = 'N'
            studentList[i][2] = grade;

        }
        console.log(studentList[i]);
    }       

}

function Input()
{
  while(true) {
    console.log("Please enter the student name (or \"end\" to end): ");
    name = readline.question("Student Name: ");
    if (name === 'end') {
      printList(studentList)
      break
    }
    console.log("Student Name is" , name);
    marks = readline.question("Enter marks for " + name + ": ");
    if (marks === 'end') {
      printList(studentList)
      break
    }
    console.log("Marks for " + name + " is " + marks );
    studentList.push([name, marks]);
  }
}
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
Samuel Tang
  • 71
  • 2
  • 8
  • 1
    It's not the problem, but: `printList` accepts a parameter called `list`, but then doesn't use it; it uses `studentList` instead. It should either not accept a parameter, or use it instead of `studentList`. – T.J. Crowder Feb 12 '20 at 10:47
  • 2
    `=>` is an arrow function, `>=` is "greater or equal" – VLAZ Feb 12 '20 at 10:52
  • @VLAZ - OMG, I completely read past that. – T.J. Crowder Feb 12 '20 at 10:53
  • 1
    @T.J.Crowder yeah...I've noticed this problem a couple of times already. We should be more watchful about similar problems because *it's so hard to spot*. I don't even fault the person who wrote that, since the syntax works (no syntax error) and it looks like it should be "equal or greater than". But it's not. It's a very annoying error because experienced users will easily overlook it - why would you have a random arrow function in a boolean expression, right? For newbie programmer, it still *looks* correct. – VLAZ Feb 12 '20 at 10:57
  • 1
    It's sort of like using `if (x = 1)` which makes sense mathematically but not programmatically. Only we've trained ourselves to look for this sort of issue. – VLAZ Feb 12 '20 at 10:58
  • Hey all, I have edited my code to everyone's and @T.J.Crowder suggestion, however it is still printing 'HD' for 'marks' no matter the number that i entered in. I've edited the code above and advise if they are still any problem with it. – Samuel Tang Feb 13 '20 at 02:04
  • @SamuelTang - On SO, it's important **not** to edit questions to fold in the changes suggested by answers. It makes the question not make sense, and makes the answers look like they just restate the question. You could *add* something to the end saying you've done what the answers said but it still hasn't worked, or just comment saying you've done that (without quoting the code). I've rolled back the edit. I've also updated my answer to address the reason you were still getting a problem and various other issues I saw in the code. Hope that helps! Happy coding! – T.J. Crowder Feb 13 '20 at 07:59
  • @T.J.Crowder I am so sorry. I did not know about not editing the question. I have read through your answers and tips. I will try going about my code with those and update again if i have any problem. For of loop for 1 thing is something new to me. I will read up the documentation for it and see how i can go by using it. Thank you very much for the help T.J! Really appreciate it. – Samuel Tang Feb 13 '20 at 11:08
  • @SamuelTang - It's my pleasure! And no worries, SO isn't quite like some other places, it takes a while to learn how things work. Happy coding! – T.J. Crowder Feb 13 '20 at 11:49

2 Answers2

6

You are using an arrow function and this is always true (if wrapped).

marks => 80

var marks = 1; // assuming a number

if ((marks <= 100) && (marks => 80)) {
    console.log('true');
}

Instead take the correct comparison sign >=

var marks = 1; // assuming a number

if (marks <= 100 && marks >= 80) { // no extra parentheses required
    console.log('true');
} else {
    console.log('false');
}
Nina Scholz
  • 376,160
  • 25
  • 347
  • 392
3

Aside from the arrow function thing Nina points out (how did I miss that?!):

You're comparing strings, not numbers. When you compare strings containing digits, the results are not the same as comparing the numbers they would evaluate to.

When building your list of students and marks, convert marks to a number. For instance:

studentList.push([name, parseFloat(marks)]);

parseFloat is just one of your options, see my answer here for a rundown of various ways to convert strings to numbers.


A couple of notes on your if/else if structure:

  • It doesn't handle the case where marks is > 100.

  • If you do them in descending order, you don't need to list the lower bound

  • There's no need to assign to studentList[i][2] in each else if block; just do it once at the end

For example:

if (marks > 100 || marks < 0)
{
    throw new Error("Invalid 'marks' value");
}
else if (marks >= 80)
{
    grade = 'HD'
} 
else if (marks >= 70) 
{
    grade = 'D'
} 
else if (marks >= 60) 
{
    grade = 'C'
} 
else if (marks >= 51) 
{
    grade = 'P'
} 
else // No `if (marks >= 0)` because we know it is, otherwise we would have throw an error above
{
    grade = 'N'
}
studentList[i][2] = grade;

There are a few other problems:

  1. You're reusing the last marks entered rather than using the marks for the specific student in the loop. You need to add

    var marks = list[i][1];
    

    inside your loop, just above the if/else if above.

  2. Your Input function should be called input (it's not a constructor function, so it shouldn't be capitalized).

  3. There's no reason name, marks, and studentList should be globals. They should be local variables in your input function. And making locals would have revealed the marks problem with printList, because you'd be trying to use the value of an undefined identifier (marks).

  4. (Almost) All new code should be written in strict mode, either by writing it inside a JavaScript module, or by having "use strict"; at the top of the script.

  5. I wouldn't use an array for the entries in studentList, I'd use an object with named properties so that you don't have to have the magic numbers 1 and 2 in the list[i][1] andlist[i][2] in printList.

  6. It's completely optional, but you could use a for-of loop rather than a for loop to loop through the array.

  7. The curly brace ({}) placement, semicolon usage, and quote choice (" vs. ') in your code is not consistent. I strongly recommend being consistent. (I also recommend writing semicolons explicitly rather than relying on automatic semicolon insertion [ASI], but there's a large minority who intentinally rely on ASI. Similarly, it seems to be all the rage at the minute to use ' rather than ", which I find nuts [I use " or template literals] but it's definitely the majority view.)

  8. Since you've used let in one place, I take it you're not constrained to using ES5 and earlier. That being the case, I recommend never using var.

  9. Really minor, but "marks is" is incorrect; it should be "mark is" or "marks are." In this code it's just a single mark, so mark would make more sense, but in the version below I've left it as marks in case that's what your assignment said to call it.

Putting all of those comments together and using a very common bracing style (a variant of Kernigan & Ritchie style often [and irritatingly] called the "one true brace style"):

"use strict";
const readline = require('readline-sync');

input();

function printList(list) {
    for (const entry of list) {
        // Get a local for `marks` via destructuring
        const {marks} = entry;
        if (marks > 100 || marks < 0) {
            throw new Error(`Invalid 'marks' value: ${marks}`);
        } else if (marks >= 80) {
            entry.grade = 'HD'
        } else if (marks >= 70) {
            entry.grade = 'D'
        } else if (marks >= 60) {
            entry.grade = 'C'
        } else if (marks >= 51) {
            entry.grade = 'P'
        } else { // No `if (marks >= 0)` because we know it is, otherwise we would have thrown an error above
            entry.grade = 'N'
        }
        console.log(entry);
    }       
}

function input() {
    const studentList = [];
    while (true) {
        console.log('Please enter the student name (or "end" to end): ');
        const name = readline.question('Student Name: ');
        if (name === 'end') {
            printList(studentList);
            break;
        }
        console.log('Student Name is' , name);
        const marks = readline.question('Enter marks for ' + name + ': ');
        if (marks === 'end') {
            printList(studentList);
            break;
        }
        console.log('Marks for ' + name + ' are ' + marks );
        studentList.push({name, marks: parseFloat(marks)});
    }
}

Using the bracing style you mostly used in your code, which is usually called the Allman or BSD style:

"use strict";
const readline = require('readline-sync');

input();

function printList(list)
{
    for (const entry of list)
    {
        // Get a local for `marks` via destructuring
        const {marks} = entry;
        if (marks > 100 || marks < 0)
        {
            throw new Error(`Invalid 'marks' value: ${marks}`);
        }
        else if (marks >= 80)
        {
            entry.grade = 'HD'
        }
        else if (marks >= 70)
        {
            entry.grade = 'D'
        }
        else if (marks >= 60)
        {
            entry.grade = 'C'
        }
        else if (marks >= 51)
        {
            entry.grade = 'P'
        }
        else // No `if (marks >= 0)` because we know it is, otherwise we would have thrown an error above
        {
            entry.grade = 'N'
        }
        console.log(entry);
    }       
}

function input() {
    const studentList = [];
    while (true)
    {
        console.log('Please enter the student name (or "end" to end): ');
        const name = readline.question('Student Name: ');
        if (name === 'end')
        {
            printList(studentList);
            break;
        }
        console.log('Student Name is' , name);
        const marks = readline.question('Enter marks for ' + name + ': ');
        if (marks === 'end')
        {
            printList(studentList);
            break;
        }
        console.log('Marks for ' + name + ' are ' + marks );
        studentList.push({name, marks: parseFloat(marks)});
    }
}

I happily used that bracing style (in C, C++, Java, C#, and then JavaScript) for something like 20 years, but just be aware that it's uncommon in JavaScript circles (and for that reason I don't use it anymore), perhaps partially because there's an ASI footgun related to it and the return keyword: If you want to return an object created via a literal, you might be tempted to do:

// DON'T DO THIS
return
{
    foo: "bar"
};

That will return undefined, because the rules of ASI say that the JavaScript engine should insert a semicolon after that return, treating it like this:

// Blech
return;
{
    foo: "bar"
};

That's easily avoided, of course, and is more a flaw in ASI than Allman style, but again, the modified K&R style is much more common in JavaScript circles than the Allman style.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875