We're going to be cleaning up some code by going through a code review on a project that's run by Cats.

Let's get started now.

What's first?

Take a look at the project as a whole to figure out how we can make changes and to figure out how it works.

First, we have a weather application here that's going to show you your current weather for whatever location, your type in.

Also, down here we just have some random html CSS, which we aren't going to worry about.

Next we have the products page, which is where we're going to spend most of our time.

We have a hanged man game here, a browser to do list and lastly, a song maker where when you click on these buttons, it'll play certain sounds for you.

And then lastly, we have about section, which is just more html CSS again, something that we're not going to worry about looking at.

So let's dive into the code to figure out how this all works.

The first thing the note is that this application is using no Jess and express to run its server.

It has the Templates folder, which contains all of the different views which are written using handle bar notation.

And we, lastly, have a public folder here, which, for the most part, is going to contain the Js, which is what we're worried about for this code review.

Also, we have a couple of utility functions inside a Tissue tells folder that are used inside of our server folder.

The first place, but I want to look at for our code review is on the home page and with our weather service.

So if we expand here a browser a little bit, scroll down to the section that's using the weather.

As we can see, we have an APB out, get weather section, and this is going to be the code that gets cold.

When we click on this.

Show me the weather button and inside of here, there's already quite a few things that we can change.

First of all, as you can see, this is using callbacks and nesting multiple call backs inside of each other, which is always a red flag for something that could be changed because it could lead to call back hell also, we're duplicating our air returns here.

As you can see, we're checking for an heir, and both times were doing the exact same thing with that error, which again is something that promises and a single weight can help you avoid before we dive too much further into this actual layout in the server.

Let's look at these two functions geo code and forecast.

These were to find inside of this utilities, so open both of these up and what's first look at the geo code and essentially all this does is take a few parameters, and then it's going to use this request library, which works a lot like fetch.

It's essentially allowing us to call in a p I and again it's returning these callbacks.

But one thing that immediately jumps out to me in this function is that this call back returns either first, an error message as the first parameter or actual results as the second parameter.

So you always have to pass undefined for one of these two options when you call the callback.

I don't really like this because it automatically makes one call back due to different things that both handles airs and it handles data.

And I'd really like to separate this into two different things, which is where promises are really going to be key.

We have the exact same problem in this forecast function, because again, we're doing the exact same thing where we have one callback that handles both the data and the actual air handling.

So I'm gonna jump to an example of where I've changed this code and show you what I would do to use promises and a single weight to clean this up.

So here's the reworked version of our app dot Js.

As you can see, we're using a sink await in order to avoid a callback health.

We're using this line here to first call our jail code function.

And then directly after that we're calling our forecast function and then sending the results back down to the client.

And as you can see, we're catching our error in one location, which means we don't have to duplicate our error code across our different callbacks, and we have it all in one place.

Also, if we go inside of our geo code or forecast function, you see, instead, we're using promises to return our results, which means that we have.

It's separate callbacks for rejecting for when we have an error and resolving for when we have successful data, which means we don't have to return undefined for air or for a data depending on the result of our actual operation.

It also means interact IGs.

We don't have to handle these two different situations.

When we were using the callback, we just used the await.

And instead of here, we're going to get the result.

Or if it fails, it'll go down and catch the error.

We did the exact same thing here with our forecast were again we split this out.

We used a promise and reject and result in order to avoid having to pass undefined through and potentially entering call back hell.

This overall was a pretty straightforward change and something that you'll see a lot in older applications before promises were created or a sink away was created.

You'll see this call back problem, and it's a really easy fix to throw in these promises and change your callbacks to either resolve or reject and then inside of your actual code, consuming it, just changing that to be flattened instead of having all of the different callbacks and using the await key word here jumping back into the old code.

I want to move on to the product section, and the very first thing we can look at is the hangman game.

Let's open that up inside of our JavaScript.

Hey man dot Js and take a look at what we have here.

First thing I noticed is that we have this class called Game and already that's a red flag for me because Game is an incredibly generic name.

This should be something like Hang Man Game or hangman or something else to denote this as a particular game instead of just a game in general.

Other than that, I like that classes are being used here because they make organizing data inside of an object much easier than using just a bunch of functions.

Another red flag that immediately jumps out to me is this dot status.

This status is set to a string here and is being checked against a bunch of different strings all over the place, and these were all strings that are exactly the same.

It either is going to be plain failed or finish.

It always is one of those three things.

As you can see in the code who's a bunch of places.

This status is being checked, and it's really easy to accidentally misspell one of these strings.

For example, you forget the eye in plain, and it's really easy to miss that, and you won't get any heirs when you run your coat.

So this is a really great example, wherein enumeration would be perfect for fixing this code.

The last thing to note about this hangman example is down here a ways we have these Gators right here, puzzle element and guess element.

And it's very misleading the name of these different variables because this puzzle element and guess element don't really correspond to what you think they would.

Also, you're using the remaining time variable identify and the your guests identify WR toe, get these things, and they don't really line up.

This I d doesn't really line up with his variable name and saying with this variable name and this i D.

So what I did in my example is I changed these names to be much more in line with what they're actually representing.

Another thing to note is that document Die query selector.

Is he used here, even though we're just queen on an I D.

So it be best to use Get element by i D instead so we could change this to get element by D and remove that hashtag and that would work it just a cz well and be slightly faster for your code.

So what's look at the example of all the different changes?

Here's the updated version of the hangman game, and the first thing you notice is I changed this class name to be hangman game because it's much easier to reason what that class is.

Also, I pulled out these different status variables into an Denham.

And since JavaScript doesn't have Denham's built into it, the easiest way to create a Denham is to create some form of constant variable, which is an object, and that object skis are going to be the value of the norm here, so plain finished and failed, and then the value of those keys are just going to be whatever your name is encoded as this way, you can never misspell the word plane here because if we go down, you can see if I'd start typing here.

Failed.

It'll automatically be populated in just like this.

And if I put something else here, I'm going to get air in the browser if I try to run this and it's spelled incorrectly.

So by having this in, um, it forces me to always have a property which is defined on an object of here.

And this is the best way to deal with these type of income strings or any form of string comparison where it's a set in stone strength.

You want to use these constant variables up here to do that.

And lastly, as I mentioned before, I renamed these variables down here so that they're much easier to reason with what they're actually representing inside of the code again, these air really straightforward changes.

The code overall was really well written.

The biggest change is this in dumb section, which I think makes the code much easier to reason with what's going on, and it makes it much more air proof in the future.

This is a great thing to put into your tool belt.

Next, back in our old application, we have the browser based to do us.

So the way this works is you can type in anything here and click.

Add new item.

It'll add it to do.

You can remove the to do at a bunch of different to dues if you want, and you can mark the rest, complete or unmarked themselves, not complete.

And also you can do some filtering up here so I can do a search.

For example, let's put in when all D's and five search you'll see it'll automatically folk about Let's form a So now let's look at the code for how this works.

It's going here to do dot Js and this code is, I think, the most complex of all the code and application ends kind of the messiest.

And I think one of the reasons for that is that there's no class is being used, and everything is just a bunch of different functions and global variables in order to determine what's going on.

So as you can see, we have a bunch of different loading and saving functions getting creating and it becomes really complex, and certain functions like get two DUIs are almost entirely useless because all they're doing is returning a variable, which is already to find up here.

So this get to do is really does absolutely nothing for us.

And there's quite a few variables that are like that in this application.

For example, there's one called Get filters just right here, and it's again returning just a variable filters.

So let's look at how I would rework this using a class in order to change some of these problems.

One of the main things is this filters method, and this filters object.

I don't really like that.

This is storing object key value pair for all the different filters.

I think this should be to find inside of a class, and the setting of the filters should be their individual own methods instead of setting the filters based on just a object.

So let's look at the new code right now so immediately.

The first thing that will jump out with you at this new code is that we're using a class for the to do list, and we have very few actual global objects.

All of this is inside of the class, and down here, the only thing that's global is our actual class that we create here, our to do list and all the different elements that we need in order to hook this up, such as these ad buttons, removed buttons, search boxes and so on.

Also, all over different document event listeners are down here.

But that's perfectly fine, because we need those to be global.

Let's go back up to the top and look at our class itself and you can see that we pass in everything we need inside of the class.

We pass in this list, which is just going to be where we put all of our elements.

We pass in our message element and our summary element.

We essentially pass in all of these different elements here that we need to use in orderto lay out our two DUIs and create our two DUIs.

Then we load our two DUIs as soon as we create this object, and this load function just gets them from local storage and saves them into this class right here so we can use these objects and persistent when we refresh the page.

Next, we have a bunch of different helper functions for getting the two DUIs storing that seduce et cetera.

And these functions are very similar to the old applications functions.

But instead of being randomly thrown throughout the application, they're all inside of one nice, coherent object which will allow us to make this really easy to know what this does this to do.

List just handles a list of two DUIs and all the functions to do so are built into the class itself instead of being throughout the entire object and the entire file.

And as you can see with the filters, I broke them out as attributes on the actual object so that they could be set and got individually instead of having to call a set function, which will try to set all of them at once or none of them at all.

So this just makes it really easy to set these different filters without having to do a bunch of complex object manipulation.

Other than that, though, the code for this stayed very similarly.

The main thing that changed was just throwing it in the class like I mentioned.

Now let's jump back and look at our very last project.

Here we go.

We have the old code open and if we scroll down.

The last project we have is this music maker, and this one's really straightforward was opened this up.

It's this middle class right here, and as you can see, this class is very straightforward.

We have a list of variables here with all of our different colors, and then when we click on a key, all we do is we just create that ball that fires across the screen and play the sound et cetera.

And there's really only one thing about this that I don't like, and that is that this color's variable is during the different color values from Marcie SS.

So we have safe sex variables called blue, yellow, red light yellow light blue, which correspond to the different colors of our buttons in our browser.

But this color list order is entirely dependent on the order of the browser buttons.

If we swap the order of these buttons in the browser, this will no longer work properly, and it won't actually send a colored ball of the right color.

So instead of determining what the color of the ball is based on these variables, I want to get the color from the actual element that the user clicks on.

So what's looking?

How I did that in the reworked version of this coat.

Here I have the new version of the code open, and as you can see immediately, that color list is completely gone, and instead here we're getting the color by going in getting the computed style of the actual key that we click on, and we're just getting the background color of that element.