Filling modal from JSON (dynamically)

Question

My Java app converts its modal object to JSON (using the GSON Library).

This is just fine, but I have to get it back into a modal in PHP. Now I've created a modal (in my case it's, for example, GeoLocation) which has several getters and setters.

Example:

class GeoLocation {

    private $cellTowerId;
    private $dateCreated;
    private $dateModified;
    private $geoLocationId;
    private $geoLocationLatitude;
    private $geoLocationLongitude;
    private $geoLocationName;
    private $geoLocationStatus;
    private $geoLocationTime;
    private $isSynch;
    private $geoLocationAccuracy;
    private $geoLocationAltitude;
    private $geoLocationBearing;
    private $geoLocationProvider;
    private $geoLocationSpeed;
    private $geoStreet;
    private $geoPostalCode;
    private $geoLocality;
    private $geoCountryName;
    private $geoCountryCode;
    private $geoFeatureName;
    private $geoPhone;

    public function getCellTowerId() {
        return $this->cellTowerId;
    }

    public function getDateCreated() {
        return $this->dateCreated;
    }

    public function getDateModified() {
        return $this->dateModified;
    }

    public function getGeoLocationId() {
        return $this->geoLocationId;
    }

    public function getGeoLocationLatitude() {
        return $this->geoLocationLatitude;
    }

    public function getGeoLocationLongitude() {
        return $this->geoLocationLongitude;
    }

    public function getGeoLocationName() {
        return $this->geoLocationName;
    }

// and many more...
}

I have a loader class which processes the JSON and should re-create the object as soon as the set<Varname>() method exists.

Are there any "security" problems so far or any way to improve it?

public function process() {
    foreach (parent::getData() as $key) {
        try {
            $geolocation = new GeoLocation();
            foreach ($key as $keyName => $keyValue) {
                if (method_exists($geolocation, "set" . ucfirst($keyName))) {
                    call_user_func(array($geolocation, "set" . ucfirst($keyName)), $keyValue);
                }
            }
        } catch (Exception $ex) {
            echo $ex->getMessage();
        }
    }
}

Since the class methods are always synchronized with the Java classes, the getter and setter methods should always exist. Is there maybe a way of doing the getters and setters dynamically using the JSON?

If there is JSON data which has no methods, like setJSONDataOfEvilStuff(), then it's not necessarily so far.

Answer 1

There's nothing wrong with what you're doing... well, not as such.
I would, however, like to stress that methods do not echo. If an exception is thrown, and all you do is catch it to echo the message, then the loop (and the rest of the code) will continue to be executed as if everything was fine.

However, you don't know if there is any code to come that will attempt to set the header(), or set a cookie or something. That code will then break (because echo has sent output to the client). This will result in lots of errors popping up all over the place, and will leave you puzzled as to what actually is the piece of code where all evil begins.

If an exception is thrown, the calling code must be notified. Let it fly, and let the registered handlers deal with it, or let the app crash.

Next: DRY (do not repeat yourself). While there is little repetition in your code, there is this one tiny block that I'd like to focus on:

if (method_exists($geolocation, "set" . ucfirst($keyName))) {
    call_user_func(array($geolocation, "set" . ucfirst($keyName)), $keyValue);
}

Within the if statement, you're calling ucfirst and concatenating a new string. Only to repeat the same function call + concatenation if the expression evaluates to true.
The way you call the method is questionable, too. call_user_func is, again, a function call that will only forward to a method call. I'd simply write the code like so:

$setter = 'set'.ucfirst($keyName);//create setter name
if (method_exists($geolocation, $setter))
{
    $geolocation->{$setter}($keyValue);//invoke method directly
}

That, to my eye, looks a lot cleaner.

More general issue
A more general issue I have with this code, is where and how you are calling these setters. What you're basically doing is constructing a new instance, and setting its properties.
It might happen that you want to set the properties of an instance that already exists, too. I, therefore, tend to create a bulk-setter on the model itself.
Because json_decode returns (on success) either an associative array or an instance of stdClass, and because both are iterable, you could add a method like this to your models:

function setByJSON($jsonData)
{
    foreach ($jsonData as $name => $val)
    {
        $setter = 'set'.ucfirst($name);
        if (method_exists($this, $setter))
            $this->{$setter}($val);
    }
    return $this;
}

This is just a basic example, though. But as luck would have it, I have an actual, real-life example of models that do exactly what you're trying to do: extract data out of the parsed json, and map the properties to their own setters. check out the setAll method of this class as an example.

The entire wrapper is still under development, though, but the models are quite handy already. Oh, and if you are fed up with creating all those pesky models for json data, check out the entire repo: the Freshdesk\Tool\Generator generates a class, complete with getters and setters for each property, and adds type-hints where possible.

Answer 2

It's secure, but it's really chunky and tedious to write. I'd do this by overriding the __call() magic method to trick GSON into thinking the methods exist, like so:

class JSONObject
{
    protected $fields = array();

    public function __call($name, $arguments)
    {
        $field = substr($name, 3);
        if(in_array($field, $this->fields))
        {
            switch(substr($name, 0, 3))
            {
                case 'get': return $this->$field;
                case 'set': $this->$field = $arguments[0]; return;
            }
        }
        throw new BadFunctionCallException("Could not find method $name");
    }
}

Then your GeoLocation class can basically just be an array of field names, like this:

class GeoLocation extends JSONObject
{
    public function __construct()
    {
        $this->fields = array('CellTowerId',        'DateCreated',          'DateModified',
                              'GeoLocationId',      'GeoLocationLatitude',  'GeoLocationLongitude',
                              'GeoLocationName',    'GeoLocationStatus',    'GeoLocationTime',
                              'IsSynch',            'GeoLocationAccuracy',  'GeoLocationAltitude',
                              'GeoLocationBearing', 'GeoLocationProvider',  'GeoLocationSpeed',
                              'GeoStreet',          'GeoPostalCode',        'GeoLocality',
                              'GeoCountryName',     'GeoCountryCode',       'GeoFeatureName',
                              'GeoPhone');
    }
}

And your object still works exactly as you'd expect:

$geo = new GeoLocation;
$geo->setIsSynch(true);
$geo->setDateModified(time());
echo $geo->getDateModified(); //1405580684
//Uncaught exception 'BadFunctionCallException' with message Could not find method setFlooglehorns
$geo->setFlooglehorns("this method doesn't exist");