Categories
  • PCRT 2
Just Some Things
  • Posts
  • Categories
  • First things first - Interacting with MySQL

    One of the first things to catch my eye in the vanilla code base of PCRT was how it interacted with the database. It's a standard PHP/MySQL setup, so nothing fancy going on there, but there was absolutely no abstraction between needing the data and getting the data. Need to fetch a work order?
    mysqli_query($mysqli, "SELECT * FROM pc_wo WHERE woid = $woid");
    Need to update the customer's name?
    mysqli_query($mysqli, "UPDATE pc_owner SET pcname = '$newname' WHERE ...");
    And so on and so forth. At first glance it seems reasonable, but there are some major issues.

    Raw calls to mysqli functions

    In PHP, calls made directly to the mysqli functions are about as low-layer as you can get when interacting with the database. The upside is that you will quickly become intimately familiar with MySQL syntax and your database's specific structure, but the downsides are numerous and severe.

    1. No tolerance for changes to the database schema (structure). Want to change a column name in the database? You also need to change any references to that column name in your code, everywhere it is referenced. For a particularly-well traveled column, you could be making changes in dozens of files/functions.
    2. Easily vulnerable to SQL Injection. You've probably heard it a million times before, even as a brand-spanking-new developer. By building your own SQL queries and executing them directly in code (in this case, via mysqli_query), the only protections you have against SQL injection are those protections you implement and utilize prior to making the call. All it takes is one unsanitized mention of ol' Bobby Tables to suddenly lose (or leak!) a lot of data.
    3. Errors may not be immediately obvious. Assuming you don't misformat the string in a way that causes PHP to throw an error, the call to mysqli_query can pass a syntactically-invalid MySQL statement to the database for it to fail silently. It's up to you to validate that the statement was executed successfully and returned some data -- manually, each and every time you do it this way.
    4. Lots of repetition: For every raw call you make, you then have to validate that you received what you wanted and extract information from the returned results. This should be happening, regardless, but it should be abstracted away. When your code needs something from the database, all it should know is that it received data (or didn't) -- all the "other stuff" should be happening elsewhere (in a class, perhaps?).

    What I did

    I will return to my preface from the previous post that I am not a professional by any means. When I began maintaining our code base, I didn't know all the best practices... heck, I wouldn't even call what I did back then "maintaining". It was more of a "hack and patch" ordeal; because of this, I'm in a constant state of optimization and refactorization as I learn new things.
    That being said, the biggest issue for me at the time was repetition. Every time data was pulled from the database, the process was: build a statement string; execute the statement; loop over the results; do something with the results. This process was inlined in the code everywhere! If I was going to start updating and adding features, I didn't want to have to do that every time... so I implemented a basic wrapper function around mysqli_query that I appropriately named query_assoc.

    The wrapper function

    What does it do? It executes the provided statement string and, if the name was any indication, returns an associative array of the results of the statement (in the case of a successful SELECT) or true or false "where appropriate". Where does my approach fall short?

    • It's still not tolerant of changes to the schema.
    • It's still vulnerable to SQL injection in the same way as above, since I'm building my statements the same way.
    • Errors still may not be immediately obvious.

    The only thing I managed to accomplish with this query_assoc function (other than an inexperienced and undeserved smug satisfaction) is that I reduced the repetition of having to manually loop through the results (or manually fetch an associative array of the results) when called. It's not where I need to be, but it did get me one step in the right direction by de-cluttering my code a bit.

    Standardized fetching

    To go along with my newly implemented query function, I began adding standardized ways to access data from the database. Want to get the same work order from the database that we did at the top of this post? Just make a call to the new get_wo_object($woid) function! Instead of writing out the entire query every time I want to pull work order information from the database, I can just pass along the work order ID ($woid) to the appropriate getter function instead. The getter function takes care of the specifics for me and either returns the work order I requested, or null otherwise.
    What does this get right?

    • It begins to standardize how I interact with the database. Instead of building the same query over and over, I can just call this function instead.
    • It is less prone to human error. When I need a work order, the only thing I need to worry about is providing the work order ID -- everything else is handled behind the scenes.

    Where does it fall short?

    • It's not quite standardized enough. After I fetch the object in question, I'm still handling an associative array representation of a row in the database, which means I'm still referencing column names directly.
    • It could be implemented better. All of my getter functions (and trust me, there are a lot) are global functions stored in a common PHP file. It works, but having classes with getters and setters would be better.

    What it should be

    Instead of implementing my own wrapper function around mysqli, I should have looked into implementing Prepared Statements or an existing implementation of prepared statements (via an ORM library). Using prepared statements curbs 2 of the 4 issues presented earlier. How so?

    • Reduces the possibility of SQL injection dramatically. Simple string concatenation is the primary entry point for SQL injections. By using a prepared statement instead, I avoid manual concatenation altogether.
    • Errors should be more obvious. Because preparing a statement, binding variables, and executing the statement happens in discrete steps, it should be much easier to catch potential issues as you're coding.

    So how do we handle the remaining two points? That's where the improvements hinted at in the "Standardized fetching" section above come into play. Instead of having a bunch of global functions for getting and setting database items, we should instead implement classes, each with their own getters and setters. This solves the remaining two issues:

    • Less repetition. This will be solved in the same way as before -- by having a standardized approach to fetching a work order, for example, I end up repeating myself less often.
    • Tolerant to schema changes. The only place column names are referenced directly is in its representative class. When I ask for a work order object, for example, I might use syntax like $wo = WorkOrder::fetch($woid). And where I would have previously accessed the work order data by directly referencing the column name- like $wo["pickupdate"]- I could instead use $wo->getPickupDate(); to completely avoid reference the schema in my code. If I change the schema, I then only have to update my code for the classes that are affected.

    Conclusion

    In summary, there's always more to learn. When I first cracked the code open to put my inexperienced fingers on it, I thought I had such a clever solution by implementing my wrapper function but -- as it turns out -- a proper solution is not that much harder to implement and it makes for better and longer-lasting code.

    Posted 8 years ago by Jesse.
  • Laying down the basics

© 2026 Just Some Things. All rights reserved.
  • RSS
  • @upccjesse
  • Admin area
  • Home