2.6 Refactoring for Clarity and Security

The first thing that I do after getting an application up and running is refactor code and there's a lot of places that we can refactor. So the first thing I look for is any repeated code. And right off the bat, I see the check for the request method and I know that we do that several places. So that would be something that we could use to refactor. So let's create a new file called app.php. This is where a lot of the common application code is going to go. Like a function that's called is_get, and it will return true or false based upon if it is a get request. So we can just grab that code and then we can paste it into our new function. And now we can use this function everywhere that we check to see if it is a get request. And immediately our code is much easier to understand just right off the bat. We of course do need to require app.php. So let's go ahead and do that. I don't think we check the request method anywhere else. No we don't. So let's go to the Delete code and let's do the same thing. We will require app.php. Here we have a check for the get request. So once again, we will use our new function there. And I think we also check for the post. Yes, we do. So we could have an is post function as well. So let's just go ahead and add that to our application code as well. So this is my process. The first thing I do is try to find where I have repeated a lot of the same code, and I try to eliminate the repetition. So once again, we'll go into Edit. Let's copy the require line once again. And then we will once again use our new is_get function. Believe we check for a POST request, we do so we will use is_post. And then finally we have the index code which we don't have anything at all, at least as far as that is concerned. So, if we wanted to, we could return to our code, and we could check to see if this does indeed still work. So, if we save this, everything looks okay. If we delete Jim, then of course he's gone. If we add him back then that should hopefully work and we will find out here in a moment. So there we go. Yes, that worked. So that first step is done. And now I want to tackle our database code because this is where the vast majority of our repetition comes from. We create this PDO object in every single file. So what I wanna do is create another folder called data. And inside of data, we will have a class. And let's just call this address book.class.php. And we can make this a class where we would create instances. But I think that this is just going to be primarily a static class where all of the methods and properties would be static. So of course we don't really need a constructor here, but we would have a private static function that we can call connect, that we would use to create our PDO object. So we can go to any one of these files. Let's go to index. First of all, let's grab where we create that PDO object. And we are just going to return that object. So that in every one of our other static methods, we will be able to call that connect method to create that database object just like this. And then we could fetch all of the records in our table, which is what we do inside of index. So then we can take the other code that we already have, and we can cut all of this out. And we do need a reference to app there but we will get that here in a moment. And we can just paste that in to the point to where we can simply just return model. And there we go. So that's inside of index, we of course want to require app.php which will also require our Address Book data class. But then we could just say model equals and then address book and then we could call the all method. And there we go we would have all of our code and that is so much easier to understand what's going on. We are retrieving all of the address book, storing that in model and we're good to go there. So let's go to app. And let's require that file. So that is data and then address book.class.php. So let's make sure that the Index page still works. It does, so we are good to go there. Although I guess I should make sure that I saved everything and I did. So we could keep on repeating this same process. Now on screen I went ahead and implemented everything else in our address book class. So we have an all a create a delete a single for retrieving a single record and update. And then of course the private Connect method. So, if we look at where that's used like in the Delete page or released the code for the Delete page. And remember that we had some kind of weird stuff going on, well it wasn't weird, it just wasn't pretty code. Well now, look at it. We don't have to worry about any database objects because all of that is being handled inside of our address book class. So, now our code is very clean. Well, it could be cleaner, it's still pretty clean, and we can at least follow what's going on. We can tell that okay, we are grabbing the ID from the get request. We're checking if Id is there and then we are getting the model. And then as far as else, we are handling the input, checking to see if everything's okay and then deleting the item. And now everything is just nice and clean. If we look at edit, there we go. So it's more of the same thing. And the next thing I want to talk about is validation, which we really haven't done at all. And that's primarily because I wanted to focus on the data, working with the database. So yes, the first name and the last name are required fields in the database, we should be checking to see if we have a first name and last name value. That of course is going to force us to refactor our code so that we could display a message to the user if they didn't supply one of those values. And really, we should do something different with the email as well. It's great that we're sanitizing it but we should use the validate email. That way, if the user disapply an email address we could validate to ensure that it is an email. But that also forces us to rethink how we are handling the email. Because if somebody enters in a wrong or, not wrong, if it's an incorrect email address. So if we just add something in here that is definitely not an email address, well the application is going to ignore it because it wasn't valid, so that email variable is empty. So therefore it set as no, and that is what gets saved in the database. So we would need to rethink how we handle that. And we're not going to do that because I want to then focus more on the next thing which is encoding your output. And this goes, hand in hand, with sanitizing the input. So there's essentially three schools of thought here. You can subscribe to whichever. It doesn't matter. But the main thing is that you are consistent. Whichever you decide to do, do it consistently. So the first school of thought is to sanitize all input. And then you can make the assumption that the data inside of your database is clean, it can be trusted. And yes for the most part, that's is true. However, that doesn't take into account that some of the data might have been manipulated directly within the database outside of the application. Or perhaps your application was compromised or the system that it's running on was compromised and a malicious data was inserted into the database. So then the second school of thought is to not really filter or sanitize the input but to encode the output. So that you basically accept whatever the user provides and then inside of the view, you encode the output with like, HTML special chars or something along those lines. And that's a perfectly valid approach as well. The third school of thought is what I subscribe to, that is to sanitize all input, and encode all output. That way, you cover all of your bases so that the input that comes through your application is sanitized and all of the output is encoded so that you protect yourself from any kind of script injection attacks. So we are going to do that and it's very simple. We're gonna add a function here inside of app.php. We'll just call it encode. And we are going to accept a model. And we are essentially going to iterate over that model. And we're going to get the key and the value, so that all we do is reset the value at the given key, but except that we are going to call HTML special chars. And there we go. And then finally, we will return model. So let's now in all of our views, all we have to do is use this encode function and we're good to go. So, inside of index php, after our foreach loop here, we could say that item = encode item. And there we go. We have encoded all of that information so that whenever we output we are outputting encoded data. As far as edit is concerned, we will essentially do the same thing. So let's just copy this so that we can just paste it everywhere else. So for the else whenever we output our form, except that in this case we need to use model because that is what we used. And there we go, we are done. And then finally inside of delete because we output data here we will once again add that call to encode. And now we are sanitizing all inputs, we are encoding all output. So we can assume that for the most part, the data in our database is clean. But just in case if it's not, we have encoded the output to protect ourselves there.