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:
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.
Your Input
function should be called input
(it's not a constructor function, so it shouldn't be capitalized).
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
).
(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.
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
.
It's completely optional, but you could use a for-of
loop rather than a for
loop to loop through the array.
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.)
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
.
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.