4
\$\begingroup\$

I currently have a design that is used to store user/selected options in a database, but I am not sure what I am doing is good or not. I need some opinion, suggestion and criticise on the design.

Pre-Condition:

  • The option is not always going to change
  • A lot of options is going to be used

Database Design:

Let's say I have a column named "country" in my user table and I store this data as an integer instead of a string (the country name). Then I use mapping to map the value to the country name in the code when I want to use it.

The reason I am doing this is because I am thinking that storing the value as an integer and retrieving the value from the integer is faster compared to a string. Furthermore, it is more or less on storage consumption.

The above is just an example; I wouldn't do this if I only need to store the user's country. I'm actually got plenty of tables that are required to store the user's option. Some of the options contain long strings such as 20 - 50 chars.

Code Mapping Design:

I created 2 Enum class: one is the base class and the other is the particular option class and I will implement the base class that help me to do the mapping.

Base Class:

abstract class BaseEnum {

    private static $constCacheArray = NULL;

    private static function getConstants() {
        if (self::$constCacheArray == NULL) {
            self::$constCacheArray = array();
        }
        $calledClass = get_called_class();
        if (!array_key_exists($calledClass, self::$constCacheArray)) {
            $reflect = new ReflectionClass($calledClass);
            self::$constCacheArray[$calledClass] = $reflect->getConstants();
        }
        return self::$constCacheArray[$calledClass];
    }

    public static function getAllOptions() {
        $options = array();
        foreach (self::getConstants() as $key => $value) {
            $options[substr($key, 1)] = $value;
        }
        return $options;
    }

    public static function getValue($key) {
        $key = '_' . $key;
        if(isset(self::getConstants()[$key])) {
            return self::getConstants()[$key];
        }
        return FALSE;
    }

    public static function isKeyExist($key) {
        $key = '_' . $key;
        if(isset(self::getConstants()[$key])) {
            return TRUE;
        }
        return FALSE;
    }

}

Country Class:

class Country extends BaseEnum {
    const _1 = 'Afghanistan';
    const _2 = 'Albania';
    const _3 = 'Bangladesh';
    const _4 = 'Brazil';
    const _5 = 'Denmark';
    const _6 = 'United Kingdom';
    ...

}

When the time I want to use to display the option into HTML, I do this:

<?php
    ...
    $options = Country::getAllOptions();
    foreach($options as $key => $value) {
        echo '<option value="'.$key.'">'.$value.'</option>'
    }
    ...
?>

When I want to display user-selected value, I do this:

 <?php
        ...
        $user = new User();
        $options = Country::getAllOptions();
        foreach($options as $key => $value) {
            if($user->getCountry() == $key) {
                echo '<option value="'.$key.'" selected="selected">'.$value.'</option>';
            } else {
                echo '<option value="'.$key.'">'.$value.'</option>';
            }

        }
        ...
    ?>

Is there any wrong with this design? Or is it better to maintain all these options in the DB? Or is there other any other better ways to do this?

\$\endgroup\$
1
\$\begingroup\$

This looks pretty good IMO. If you are considering taking these values to the database, make sure you create a separate table for each category. Do not use enum type columns.

However there are few minor things I noticed that you can refactor.

  • Initialize $constCacheArray as an array so you can avoid the first 3 lines in getConstants()

  • isKeyExist method can be a single line

    public static function isKeyExist($key) {
        return (isset(self::getConstants()['_' . $key]);
    }
    

    getValue method has similar logic too but for readability's sake you can keep it as it is.

  • Avoid method calls inside loops. Take $user->getCountry() out of the foreach and assign it to variable which you will use for comparison.

  • Also to remove the _s from the keys you can use a regex. If you plan to use string keys for enums (eg: const disabled = 'Disabled';), this will come in handy.

    json_decode(preg_replace('/"_([0-9]*)":/i', '"$1":', json_encode($arr)), true)

\$\endgroup\$

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.