Implicit behaviour of Jelly::select() and ->load()

[gerasim]

First. Jelly::select('modelname')->load($key); If $key == null or 0, is equal for

Jelly::select('modelname')
             /* ->where(':primary_key', '=', 1) */(i.e. without "where")
             ->limit(1)
             ->execute();

As a result, if $key == null or 0, we have one LOADED object. I think that in this case we must get NOT loaded object.

I suggest this changes:

public function load($key = NULL)
{
    if ($this->_type === Database::SELECT)
    {
        if (!$key)
        {
            return Jelly::factory($model);    
        }  

        $this->where(':unique_key', '=', $key);       

        return $this->limit(1)->execute();
    }

    return $this;
}

Second. Jelly::select('modelname', $key); I think that if the key is set explicitly (even if $key == 0), the developer expects to get object of Jelly_Model (let even empty) instead of Jelly_Builder

I suggest this changes:

//class Jelly
public static function select($model, $key = NULL)
{
    $builder = Jelly::builder($model, Database::SELECT);

    if ($key)
    {
        return $builder->load($key);
    }
    elseif (0 === $key)
    {
        return Jelly::factory($model);
    }

    return $builder;
}

[jonathangeiger]

I agree. For the second example I think this might be better:

//class Jelly
public static function select($model, $key = NULL)
{
    $builder = Jelly::builder($model, Database::SELECT);

    if (func_num_args() === 2)
    {
        return $builder->load($key);
    }

    return $builder;
}

[gerasim]

I agree. It's more correctly

[jonathangeiger]

Closed by 7f6cb1978c2b3164e6ea9aa5b18cc8e51c6cc350. Thanks gerasim!

[aron]

I'm not 100% sure this is the correct behaviour. I've been using this method to load a single row. It was working just fine until this update now I'm returned an unloaded model.

For example:

$user = Jelly::select('user')->where('name', '=', 'bill')->load();

Running this used to return a loaded user object (if the row existed in the DB). Now I get an empty model. Calling load() on a query builder should load the object. The old behaviour seems correct to me.

My comment only applies to the changes to Jelly_Builder::load() the change to Jelly::select() seems sound enough.

Cheers,

Aron