1

Pre: I am looking at a quite big PHP project developed by a company which we need to partly interact with while building a new webapp.
The whole application is developed in PHP 5.

Most of the classes in this project, and in particular all the classes that interact with the database, are declared abstract and contain only static functions.

Take for example this file:

abstract class DataHelper_Command
{

    /**
     * @param unknown $id_device
     * @param unknown $latest_command_id
     * @return Command[]
     */
    public static function getCommandsToSend($id_device, $latest_command_id)
    {
        $rows = DBUtility::executePreparedQuery('SPS_COMMANDS_TO_SEND', array(":id_device" => $id_device, ":latest_command_id" => $latest_command_id));
        return DataHelper_Command::commandsFromDataTable($rows);
    }

    public static function getFW()
    {
        $rows = DBUtility::executePreparedQuery('SPS_FW_VERSION');
        return $rows;
    }

    /**
     * @param unknown $rows
     * @return Command[]
     */
    private static function commandsFromDataTable(&$rows)
    {
        $res = array();
        foreach ($rows as &$row)
        {
            $res[] = DataHelper_Command::commandFromDataRow($row);
        }

        return $res;
    }

    /**
     * @param array $row
     * @return Command
     */
    private static function commandFromDataRow(&$row)
    {
        return new Command($row["id_command"], $row["type"], $row["text"]);
    }
}

I don't even see any trace of using a Singleton pattern in the project!

That sounds a bit strange to me, as I always heard that using static classes is a nightmare in terms of performance.
This server was meant to be able to run on both standard webservers and on a really performance-limited embedded device (An ARM running at 400MHz, with 128 MB of RAM).

Am I missing something here? Are there any use-cases in which this is considered a good practice?

3
  • Do you know the exact PHP version the project was build for? I do remember static methods having a somewhat significant performance penalty some time ago, but I don't think that's the case anymore. Also, singleton? Bleh! – yannis Dec 9 '15 at 14:42
  • @Yannis yeah, that's written in PHP 5.3 – GavinoGrifoni Dec 9 '15 at 14:43
  • 1
    commandFromDataRow(&$row) The ampersand indicates this code probably dates back to php 4. Still no excuse for this style of programing as php 4 objects worked fine. But dependency injection type stuff really was not all that popular in php 15 years ago. – Cerad Dec 9 '15 at 15:30
3

Performance is not even the biggest problem here, secret dependencies and lying APIs are.

From a testable point of view, static methods are awful, because they usually represent global state, which, surprisingly, is awful as well, because it means if you run a function two times, you can get different results even when the inputs are the same (that is what global state does). How do you test that?

I cannot really see any case, where I would prefer static methods instead of normal ones. If you can, do not use them, if you must use them, your design is most likely wrong and needs refactoring.

Check Clean Code Talks by Miško Hevery from 2008, they are still relevant and provide great explanations on why static methods and singletons should be avoided.


Some youtube references to some talks by Miško Hevery:

2
  • "if you must use them, your design is most likely wrong and needs refactoring" my exact thought here: I know that the development of this elephant took 2 years, so my suspect is they continued building over something without having a clear design done at the beginning – GavinoGrifoni Dec 9 '15 at 14:47
  • 2
    @GavinoGrifoni Sadly that happens in production. Sometimes you do not have the budget to do it all again correctly. Even for systems which are inherently wrong, if I end up adding a new module to that system, I do it the right way. So at least something is designed correctly. I recommend you to do the same. Use the ugly elephant, but make your baby beautiful. – Andy Dec 9 '15 at 14:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.