3
\$\begingroup\$

I am just looking for code improvements as I believe that it is done, and would like to see it expanded.

I mainly want code improvements to make it more efficient, but any and all constructive criticisms are enjoyed

If the snippet is broken the link to the fiddle is here:

https://jsfiddle.net/BOSS2342/q18z037w/

What my code does is it take the two inputs, Current and Total Grade, and it uses that to give you some information about it, such as your letter grade and grade percentage, along with the amount you need to go up and down a letter grade.

document.addEventListener("keyup", function(event) { //Used so you can press enter if (event.keyCode === 13) { Calc() } }); function Calc() { var g = /\d+$/.test(document.getElementById('g').value) ? Math.round(parseFloat(document.getElementById('g').value) * 100) / 100 : `You did not input your Current Points correctly`, //Testing and getting the value of Current Grade t = /\d+$/.test(document.getElementById('t').value) ? Math.round(parseFloat(document.getElementById('t').value) * 100) / 100 : `You did not input your Total Points correctly`, // Getting and testing the value of Total Grade m = Math.round((g / t) * 10000) / 100, //Using this for percentage calculations n = [], //empty var for later grade = (m) => { return m >= 90 ? 'A' : m < 90 && m >= 80 ? 'B' : m < 80 && m >= 70 ? 'C' : m < 70 && m >= 60 ? 'D' : 'F' }, //Used to convert Percentage into letter grade x = grade(m), cgrade = (m) => { return m == 'A' ? 90 : m == 'B' ? 80 : m == 'C' ? 70 : m == 'D' ? 60 : 50 } //Does the inverse of that for (i = 0; i <= t; i++) { n.push(Math.round(i / t * 10000) / 100) } //This is used to find all the possible percentages with the Total Grade findProx = (m) => { c = n[0] d = Math.abs(m - c) for (i = 0; i < n.length; i++) { if (n[i] === m) { return i } f = Math.abs(m - n[i]) if (f < d) { d = f c = i } } return c; } //Used to find the closest to the number I want var down = x == 'F' ? g : cgrade(x) == g / t * 100 || g == t ? Math.round((g - findProx(cgrade(x)) + 1)*10)/10 : Math.round((g - findProx(cgrade(x)))*10)/10, //Used to find the closest letter grade down, this is just a lot of if statements up = x == 'A' ? t - g : Math.round((Math.abs(g - findProx(cgrade(x+10))))*10)/10 //Used to find the closet letter grade up, this is just one more if statement document.getElementById('r1').innerHTML = typeof(g) == 'string' || typeof(t) == 'string' ? `Error` : `Your grade is: ${x} ${m}%` //Used to display the Grade and Percentage, along with an error document.getElementById('r2').innerHTML = typeof(g) == 'string' && typeof(t) == 'string' ? `You did not input either options correctly` : typeof(g) == 'string' ? g : typeof(t) == 'string' ? t : Math.sign(up) == -1 ? `You have ${Math.round((g-t)*100)/100} points of extra credit` : g == t ? `You have a 100% congrats!` : x == 'A' ? `If you gain ${Math.round((t-g)*100)/100} points you will have a 100%` : `If you gain ${up} points you'll go up to the next letter grade: ${grade(m+10)}` // A lot of stuff used for user information for the amount of points you need to go up a letter grade document.getElementById('r3').innerHTML = typeof(t) == 'string' || typeof(g) == 'string' ? `Reinput and try again` : m == 100 ? `If you lose ${down} you will have a 90%` : Math.sign(up) == -1 ? `If you lose ${Math.abs(Math.round((t-g)*100)/100)} points you will have a 100%` : g == 0 ? `You have no points, I hope you get that up :D` : `${g == 69 && x == 'F' || g == 420 && x == 'F' ? `Nice i` : `I`}f you lose ${down === 0 ? 1 : down} points you'll ${x == 'F' ? `have a zero`:`go down to the next letter grade ${grade(m-10)}`}` //A lot of stuff used for user information for the amount of points you need to go down a letter grade. } document.getElementById("btn").onclick = function() { document.getElementById('m').style.display = "block" } document.getElementsByClassName("close")[0].onclick = function() { document.getElementById('m').style.display = "none"; } window.onclick = function(event) { if (event.target == document.getElementById('m')) { document.getElementById('m').style.display = "none"; } }
 body { background-color: #f0efda; } .inputs { display: flex; justify-content: center; } .footnote { font-size: 12px; user-select: none; color: slategray; position: fixed; bottom: 0; left: 5px; } p { display: flex; justify-content: center; } input { width: 100px; height: 20px; } .input { line-height: 50px; display: inline-block; } .grid { background: #333; display: grid; text-align: center; grid-template-columns: 75% 1fr; width: 95%; position: fixed; top: 5px; padding: 10px; } .grid b { text-align: left; user-select: none; border-right: 1px solid #bbb; font-size: 40px; color: white; } .grid a { display: flex; margin-left: 5%; padding-top: 12.5px; color: white; user-select: none; justify-content: center; align-content: center; } .grid a:hover { cursor: pointer; background-color: #111; } .modal { background-color: rgb(0, 0, 0); background-color: rgba(0, 0, 0, 0.4); display: none; position: fixed; z-index: 1; padding-top: 100px; left: 0; top: 0; width: 100%; height: 100%; overflow: auto; } .modal-header { background: #333; display: grid; grid-template-columns: 90% 1fr; padding: 1px; } .modal-header span { display: flex; align-content: center; justify-content: center; } .modal-content { background-color: #fefefe; margin: auto; user-select: none; padding: 20px; border: 1px solid #888; width: 80%; } .close { color: white; display: flex; margin-top: 7px; margin-right: 7px; font-size: 28px; font-weight: bold; } .close:hover, .close:focus { color: #000; text-decoration: none; cursor: pointer; } .button { background-color: #4CAF50; border: none; color: white; padding: 15px 32px; text-align: center; text-decoration: none; display: inline-block; font-size: 16px; transition-duration: 0.4s; } .button:hover { box-shadow: 0 12px 16px 0 rgba(0, 0, 0, 0.24), 0 17px 50px 0 rgba(0, 0, 0, 0.19); } .button2 { background-color: #4CAF50; border: none; color: white; padding: 15px 32px; text-align: center; text-decoration: none; display: inline-block; font-size: 16px; transition-duration: 0.4s; } .info { color: white; border-right: 1px solid #bbb; margin: 5px; margin-left: 10px; text-align: left; font-size: 30px; }
 <body> <div class='inputs'> <div class='grid'> <b>Grade Calculator</b> <a id="btn">Info</a> </div> </div> <br><br><br><br> <div class='inputs'> <button class='button' onClick="Calc()" width="50px">Calculate</button> </div> <br> <div class='inputs'> <input class='align' type='text' id="g" autocomplete="off" maxlength="9" placeholder="Current Points"> / <input type='text' id="t" autocomplete="off" maxlength="9" placeholder="Total Points"> </div> <p id="r1"> This is your grade </p> <p id="r2"> If you gain this many points you'll go up to the next letter grade: </p> <p id="r3"> If you lose this many points you'll go down to the next letter grade: </p> <div id='m' class='modal'> <div class="modal-content"> <div class = 'modal-header'> <b class='info'b>Info</b> <span class="close">&times;</span> </div> <p class='input'>Put your current points in your class in the <input placeholder='Current Points' disabled='disabled'> box, and your class's total points in the <input placeholder='Total Points' disabled='disabled'> box. Then click <button class='button2' disabled='disabled'>Calculate</button> or enter. It will give you how many points you need to go up, or down a letter grade. </p> </div> </div> <div class='footnote'> <p> Hello I am Zain, I made this calculator for you! Full Release </p> </div> </body>

\$\endgroup\$
9
  • \$\begingroup\$ Ah so my question is going to be lost into the pile of lost questions in stack exchange, because of a slight rounding issue, that not only you won't help with, but also that you felt proper enough to say that this program doesn't work after finding one bug. You don't stop playing games after finding one but, and you can tell that it says student next to my name, so instead of just leaving me in the dust and I never get any improvements in general, you also gave no constructive criticism. You just copy and pasted the end of your response from 10 other people you did this to. \$\endgroup\$ Commented Dec 13, 2021 at 15:04
  • \$\begingroup\$ And I know it is my job to fix this, but I have no idea how, so my options are to, go to stack overflow to never get an answer for this ever. Or hope that some kind soul will stop my question from being flagged and then taken down. I have no idea how to get better and you being the 5k rep that you are can shut this down at any time. So either help me with anything or maybe just never see this site again. There will be a huge block in the future programmer, if every learner has to fix everything before anything ever gets accomplished. \$\endgroup\$ Commented Dec 13, 2021 at 15:07
  • \$\begingroup\$ I am sorry. No, I didn't copy and paste any part of my comment. My idea was indeed to give you the opportunity to fix this bug. As you have noticed I didn't close your question. Try to keep a positive attitude when using the internet. Sometimes things come across quite different from how they were intended. Ah, you fixed it. See, that wasn't so hard. \$\endgroup\$ Commented Dec 13, 2021 at 15:18
  • \$\begingroup\$ I understand what you mean, and I was quite frustrated as I was scared my question would be lost, as I have had many on stack overflow that were. I have lost many hours due to this website, and sometimes it makes me wanna quit. \$\endgroup\$ Commented Dec 13, 2021 at 15:20
  • \$\begingroup\$ Yes, I can understand that completely. I have had my own frustrating experiences. Questions never seem good enough. Or when spending time to answer a question and finding it was just deleted when sending it in. Are you sure you fixed the problem? I tried your jsfiddle, then copied the JS from your question, but I still cannot get it to work. \$\endgroup\$ Commented Dec 13, 2021 at 15:23

2 Answers 2

Reset to default
2
\$\begingroup\$

General points

  • Use number input for numbers. The DOM interface HTMLInputElement means you don't have to worry about vetting for anything but numbers
  • Keep code out of the global scope. Wrap it in an IIFE (Immediately Invoked Function Expression)
  • Run code in strict mode to avoid some dangerous problems your code has. Eg undeclared variables.
  • Avoid repeating data. You have the data for conversion from percent to grade twice. This make the data highly prone to error.

Design.

Make the user experience as simple as possible. The less the user must do to get a result the better the experience.

In the example the grade and percent is calculated automatically. There is no need for the user to click a calculate button.

It will never display an error as it considers bad (unknown values) as an incomplete form and simply waits for the data to be completed rather than force more user interaction,

Be careful with JavaScript numbers and don't report more or less precision than required.

Don't allow inputs to hold values that do not make sense. Max Score can never be lower than score. Score can never be greater that max score or less than 0. Max score can never be lower than 1.

Example

A simple example show how one can achieve the same result as your code. Note that I did not bother styling the content.

I have added a few comments to explain what you may be unfamiliar with.

;(()=> { // IIFE Function is isolate code from global scope. "use strict"; // Use strict mode const els = {scoreInEl: null, maxInEl: null, percentEl: null, gradeEl: null}; Object.keys(els).reduce((o, k) => (o[k] = document.querySelector("#"+k), o), els); // Required data once. const grades = [[0.9, "A"], [0.8, "B"], [0.7, "C"], [0.6, "D"], [0, "F"]]; els.scoreInEl.addEventListener("input", update); els.maxInEl.addEventListener("input", update); function calcGrade(score, maxPoints) { const frac = score / maxPoints; // Calculate number of digits to use when rounding the percentage const digits = (Math.max(2, Math.ceil(Math.log10(maxPoints))) - 2) | 0; const level = grades.find(g => frac >= g[0] || g[0] === 0)[1]; return {percent: (frac * 100).toFixed(digits) + "%", level }; } function update() { var score = els.scoreInEl.valueAsNumber, max = els.maxInEl.valueAsNumber; if (!(isNaN(score) || isNaN(max)) && max > 0) { if (score > max) { els.maxInEl.value = max = score } // Ensure max >= score const g = calcGrade(score, max); els.percentEl.textContent = g.percent; els.gradeEl.textContent = g.level; } } })();
input { width: 70px; }
<label for="scoreInEl">Score........:</label> <input type="number" id="scoreInEl" min=0 maxLength=5 placeholder="Score"><br> <label for="maxInEl">Max Score:</label> <input type="number" id="maxInEl" min=1 width=5 placeholder="Max Score"> <div> Percent: <span id="percentEl"></span></div> <div> Grade..: <span id="gradeEl"></span></div>

Second example same but uses ids to directly reference elements.

;(()=> { "use strict"; const grades = [[0.9, "A"], [0.8, "B"], [0.7, "C"], [0.6, "D"], [0, "F"]]; scoreInEl.addEventListener("input", update); maxInEl.addEventListener("input", update); function calcGrade(score, maxPoints) { const frac = score / maxPoints; const digits = (Math.max(2, Math.ceil(Math.log10(maxPoints))) - 2) | 0; const level = grades.find(g => frac >= g[0] || g[0] === 0)[1]; return {percent: (frac * 100).toFixed(digits) + "%", level}; } function update() { var score = scoreInEl.valueAsNumber, max = maxInEl.valueAsNumber; if (!(isNaN(score) || isNaN(max)) && max > 0) { if (score > max) { maxInEl.value = max = score } // Ensure max >= score const g = calcGrade(score, max); percentEl.textContent = g.percent; gradeEl.textContent = g.level; } } })();
input { width: 70px; }
<label for="scoreInEl">Score........:</label> <input type="number" id="scoreInEl" min=0 maxLength=5 placeholder="Score"><br> <label for="maxInEl">Max Score:</label> <input type="number" id="maxInEl" min=1 width=5 placeholder="Max Score"> <div> Percent: <span id="percentEl"></span></div> <div> Grade..: <span id="gradeEl"></span></div>

\$\endgroup\$
3
  • \$\begingroup\$ Wow I love this a lot. This is really good, my biggest complaint about this is I do like to calculate extra credit, but this is wonderful input, and I will be trying to implement all of these. Also something I noticed is how you used the MVC model in there. I recently learned about it in a different answer. And it is something I would like to implement. \$\endgroup\$ Commented Dec 14, 2021 at 15:10
  • \$\begingroup\$ I am really bad with Jquery I think, so what would be a great thing to find the documentation for like the .textContent or the .valueAsNumber? \$\endgroup\$ Commented Dec 14, 2021 at 15:14
  • \$\begingroup\$ @ZainWilson-WCHStudent I provided a few links to MDN at top of answer. MDN is a good up to date reference to all things web related. Yo will find what you are looking for there. \$\endgroup\$ Commented Dec 14, 2021 at 15:38
2
\$\begingroup\$

A quick review;

  • This code does not follow MVC. The worst case is where Calc, gets data from the form, calculates, and then sets data on the form. The function name is basically misleading.

  • 13 is a magical number, and could have been a nicely named constant

  • You should consider using const and let instead of var

  • Single character variables like c,n,m,x make the code too hard to parse

  • I find nested ternary statements too hard to maintain. A funny side note is that there is a bug where I cant get a 'D', but the ternary is too much to analyze.

  • You should consider using semicolons at the end of your statement in 20 locations

  • You have five globals that you do not declare (I, findProx, c, d, f)

  • onClick="Calc()" is considered bad practice, I would wire this with addEventListener

  • document.getElementById("btn").onclick is also considered bad practice, I would wire this with addEventListener as well

  • r1, r2, r3 are terrible names, I suggest gradeDescription, pointsNextGrade, and pointsPreviousGrade

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for this, I have never heard of MVC, I will look into that. I will implement a lot of these changes. \$\endgroup\$ Commented Dec 14, 2021 at 14:55

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.