gbaja / mentalmathsgame

Simple mental maths game

Mental maths game

Time based mental maths game- numbers are between 10 and 20

mentalmathsgame's Issues

Code review

Hey Fatimat, here is my code review. Don't feel like you need to make these changes but hopefully some of my comments are useful for your future projects.

• There are lots of ids that could be classes. I think it's generally accepted to always use classes unless you have to use ids. As I understand it, ids have a greater 'specificity' than classes so if you are using the ids for Javascript and CSS sometimes it can get tricky when styling things as the id will always have precedence over other classes. I didn't properly understand specificity until quite late, this page is helpful for this: https://developer.mozilla.org/en/docs/Web/CSS/Specificity. This thread is good too, which suggests that you shouldn't use ids for css styling (unless you must): https://stackoverflow.com/questions/8279132/why-shouldnt-i-use-id-selectors-in-css.

• In some places you have used tags without classes e.g. <h1>Mental Maths Quiz</h1> although the different h tags will display the fonts at different sizes it's best practice to add your own styling to them. I believe that different browsers sometimes display them differently so it's best to override the default with your own styles.

• on line 25 you have onclick='window.location.reload()' - It's best practice not to have inline JS functions in your html. Instead you could have something like this in your JS file:

var playAgainButton = document.getElementsByClassName("playAgain");
playAgainButton.addEventListener('click', refreshPage);

function refreshPage(){
    window.location.reload();
};

• Global variables. At the top of your file you have lots of variables, which is fine, but at the moment they are global variables. These can be dangerous if the project was bigger and you had lots of JS files. If two people accidentally called a global variable the same thing then your site might start behaving weirdly. This is not super important at this stage but it's something to be aware of (it will come up at Founders and Coders). The most common way to stop variables from being global is to wrap all the JS in an IIFE (Immediately Invoked Function Expression), which looks like this:

(function () {
    // logic here
})();

When the JS file is run the IIFE is invoked straight away so it doesn't affect the logic, however all the variables inside are scoped to the function instead of the window/document (so they are not global).

• add a title to the at the top of the html doc. That way it will appear on the tab in your browser!
  

• Its a matter of choice but it's generally accepted to put a space between the css attribute and curly bracket so .name { rather than .name{.

• When the value is 0 in CSS you don't need the px. So bottom: 0; instead of bottom: 0px; This doesn't make much difference but it makes less code and a bit cleaner.