CRUD and OOD. How to approach this?

Question

Please be brutally honest, and tear my work apart if you have to.

So I'm re-writing a small web application that I recently made. The reason for this is simply that the code got pretty messy and I want to learn and apply better OO design. What this application should do is just simple CRUD. I have a database with 3 tables, companies and partners which are in no relation to each other, and city which has a 1:n relation with companies and partners. Very simple, really. Now, I have several questions which I will state at the end of my post. Here I'll just try to explain:

My first approach was that I created classes company, partner, and city, fetched all datasets from the database, and created objects from that:

class company {

    private $id   = null;
    private $name = null;
    private $city = null;

    //many more attributes
    
    function __construct( $id, $name, $city, [...] ) {

        $this->id   = $id;
        $this->name = $name;
        $this->city = $city;

        //huge constructor
    }

   /*
    *  getters + setters here
    *
    *  no need to paste the partner class as it looks just like this one
    *
    */
}

And that is all these classes did. I fetched every dataset from the database and constructed company, partner, and city objects (the attribute city within these classes is an object with several attributes itself) and saved them into two arrays arr_companies and arr_partners, which then held these objects...and it worked fine like that.

Now, what I wanted is to update, insert, delete into the database, and all 3 classes (city, company, partner) need this functionality. My approach was that I created a new class with a constructor that would basically take 2 strings command and object, e.g. ('update', 'company') and it would then update the company directly in the database leaving my objects untouched. That made me really sad because I had such nicely constructed objects and I didn't know how to make use of them.

Questions:

• Is it bad to have such huge constructors (my biggest one would take 28 parameters)?

• Should you have a separate class for database operations or is it better to have maybe an abstract class or interface for it and let the subclasses themselves handle update, delete, and insert?

• Is it common to just write, and delete from the database whenever, or should I just apply these changes to my objects and only execute the commands to the database later, for example when the session ends?

• I figure an application like this must have been done a fantastillion times before. What is the proper approach here? create objects, work with objects, and save them to the database?

• I have so many questions but I think many of them I just don't know how to ask.

Please note that if possible I would not like to use an ORM at this point.

Thank you very much for your time.

Answer 1

You are essentially writing your own ORM. So, I wouldn't discount just switching to one that's already been written for you. The advantage to rolling your own is that you gain an understanding of how it works as your write it. But the disadvantage is that someone else has probably already done it better. But assuming you want to continue on...

General Advice: Remember to always break the problem down into simpler and simpler pieces. Each class should only perform a simple function. Also, you should not have to worry about caching updates... unless perhaps your database is on the other end of a remote connection over a modem.

Specific Advice follows:

I would setup your entity instance classes to house data and not to do a lot of data loading. Use other classes and logic for loading the data. I would use the constructor of the entity class only to populate the data that pertains to the class (and it's children).

A simple thing to do is to use static methods on the entity class for loading and saving data. E.g.

class city {

    private $id   = null;
    private $name = null;

    function __construct( $id, $name ) {
        $this->id   = $id;
        $this->name = $name;
    }

    // getters and setters
    ...

    // ---------------------
    // static functions
    // ---------------------

    public static function loadById($cityId) {
        // pull up the city by id
        $retval = new city(row["id"], row["name"]);
        // close db connection
        return $retval;
    }

    public static function loadByCustomerId($customerId) {
        // pull up multiple cities by customer id
        // loop through each row and make a new city object
        // return a hash or array of cities
    }

    public static function update($city) {
        // generate your update statement with $city->values
    }

    // other methods for inserting and deleting cities
    ...
}

So now the code to get and update cities would look something like this:

// loading city data
$city = city::loadById(1); // returns a city instance
$cities = city::loadByCustomerId(1); // returns an array of city instances

// updating city data
$city->name = "Chicago"; // was "chicago"
city::update($city); // saves the change we made to $city

The static methods are not the best way to implement this, but it gets you pointed in the right direction. A repository pattern would be better, but that's beyond the scope of this one answer. I find that often I don't see the merit in a more involved solution like the repository pattern until I run into problems with the simpler solutions.

Answer 2

Questions posed in OP:

"Is it bad to have such huge constructors (my biggest one would take 28 parameters)"?

• Yes. Imagine the calling code. You would have to pass 28 different values, not to mention each call would have to respect the exact order specified in the constructor. If one parameter was out of place, you could wreck havoc with parameter dependent algorithms. If you really need to pass a large number of parameters, I would recommend passing them in as an array (posted an example to another SO question).

"Should you have a separate class for database operations or is it better to have maybe an abstract class or interface for it and let the subclasses themselves handle update, delete, insert?"

• Generally speaking, when creating classes, you want to try to identify the nouns that best represent your business needs. In your specific case you would probably have three classes; Company, Partner, and City.

• Now within each class (noun), your methods would be in the form of verbs, so symantically your calling code makes sense: if ($company->getName() === 'forbes')

• As you mentioned, each class needs a database object (dbo) to work with, so you could implement any number of patterns to expose datase connections to your classes; singleton, singleton with factory, or dependency injection, etc.

• Abstract (parent) classes are great for sharing common algorithms across child classes, and should be identified when you are in the psuedo-code stage of your design. Parent classes also allow you to force child classes to have methods by declaring abstract methods within the parent.

• Interfaces are a useful tool in certain situations, but I find they are less flexible than declaring abstract methods in parent class. But are good in situations where classes do not share a common parent.

"Is it common to just write, delete from the database whenever or should I just apply these changes to my objects and only execute the commands to the database later, for example when the session ends"?

• CRUD activity should happen at the time the action is executed. If you wait for the session to end, you may run into situations where a session is pre-maturely ended due to a user closing a browser, for example. To better protect your data you can wrap your CRUD activity within transactions.

• If you are running a high-traffic application, you can implement a queuing system and queue up the work to be done.

"I figure an application like this must have been done a fantastillion times before. What is the proper approach here? create objects, work with objects, save them to the database"?

• You are correct, this has been done before, and are commonly referred to as ORMs (object relation mappers). Basically, an ORM will introspect your database schema, and create objects (and relations) which represent your schema. So instead of working with native SQL, you are working with objects. Although you can use SQL for custom business needs, but in the case of Doctrine, you would use Doctrine Query Language (DQL) vs native SQL.

• An ORM I would highly recommend is Doctrine.

If you do not want to use an ORM, you can add CRUD methods to your primary classes. I Opted for an interface so your classes don't have to extend from a parent comprised of database operations. Also, check out this post on using a singleton/factory for exposing your classes database object(s).

Consider the following:

// Company.php
class Company implements iDatabaseOperation

    public function delete()
    {
        // Lets use a DBO singleton/factory for DB access
        //   Uses PDO, which is strongly recommended
        $dbo = Database::factory(Database::DATABASE_NAME);

        $dbo->beginTransaction();

        try {

            $sql = 
                "DELETE FROM " .
                "    company " .
                "WHERE " .
                "    id = :companyId " .
                "LIMIT 1";

            $stmt = $dbo->prepare($sql);

            $stmt->bindValue(':companyId', $this->getId());

            $stmt->execute();

            $dbo->commit();

        } catch (Exception $e) {

            $dbo->rollback();

            error_log($e->getMessage();

            $e = null; // Php's garbage collection sucks
        }
    }
}

// iDatabaseOperation.php
interface iDatabaseOperation
{
    public function delete();
    public function update();
    public function insert();
}

Answer 3

You might want to look at ruby on rails.

You don't necessarily have to switch over to it, but look at how they implement the MVC pattern and achieve CRUD.

Answer 4

1. It is realy bad. Code is completele unreadable in this case. You have options
   • to use setters (can add validation logic inside, better readability, no need to fill empty fields with null)
   • to have separate class builder for each domain class (takes some memory for additional object). Example in java hope you can understand:

     class CompanyBuilder {
     private final Company c;
     public CompanyBuilder() {
      c = new Company();
     
     }
     CompanyBuilder addId(String id){c.id = id;}
     // id should be package visible and class should be located in the same package with builder
     CompanyBuilder addName(String name){...}
     CompanyBuilder addCity(String city){...}
     Company build(){ return c;}
     }
     

   • hybrid solution to have methods to organise chain(worse debugging, better readability). In java will be methods:

     class Company {
     ...
     Company addId(String id){
     this.id = id;
     return this;
     
     }
     Company addName(String name){...}
     ...
     }
     Usage: 
     Company c = new Company().addId("1").addName("Name1");
     

   • maybe you can create more granular objects to reuse them later and add specific logic in correct place. For instance it can be Address(Location) object for company.
2. Follow single responsibility principle. SOLID description on wiki. It helps to change database specific code without affection of other part of system in your case. Well, separate domain and database specific code, have common interface or abstract class(if you have common logic for all of domain classes - liskov principle). In subclasses implement domain specific part.
3. If you do not want to lose data you should save them each time or have cluster of servers or have distributed cache. If it is ok to lose save them in the end of session as batch. It will increase youre performance. Also you should save in transaction each time if you have concurrent updates.
4. Approach is get data from database/construct objects from this data or new objects/ work(update) objects/write data from objects to database
5. just write more code and read stackoverflow

Finally I suggest to read "Clean Code: A Handbook of Agile Software Craftsmanship" R.Martin.

Answer 5

I think a constructor with 28 parameters is too much, you should others classes managing some attributes having some stuff in common. You should give us what kind of others attributes you instanciated, and it could help you to find a way to make more common objects.

I think you should also create a class managing the operations and the database like a DBHandler with delete, update, and so on.. In my opinion, do modifications on tuples in your database directly after functions are called are important.

Why? Because it could avoid conflict, like the case you try to update an object which is supposed to be deleted for example, if you do modifications on your database at the end.

Answer 6

What you are doing looks greate. What you can add is an intermediate layer which maps your business object to your database(object relation mapping). There are a lot of object relational mapping api out there. Check this wikipedia list for ones you can use for PHP