Check 'present' methods in isset() implementation

[BenConstable]

I've updated the isset() and offsetExists() methods to check for the existence of present{$VariableName} methods and their result, meaning that you can now do:

<?php

class MyObject {

    public $firstname = 'First';
    public $lastname = 'Last';
}

class MyPresenter extends \Robbo\Presenter\Presenter {

    public function presentFullname()
    {
        return $this->firstname . ' ' . $this->lastname;
    }

    public function presentNothing()
    {
        return null;
    }
}

$presenter = new MyPresenter(new MyObject());

isset($presenter->firstname); // true
isset($presenter->lastname);  // true
isset($presenter->fullname);  // true
isset($presenter->nothing);   // false
isset($presenter->notset);    // false

This also works with arrays:

<?php

class MyPresenter extends \Robbo\Presenter\Presenter {

    public function presentFullname()
    {
        return $this['firstname'] . ' ' . $this['lastname'];
    }
}

$presenter = new MyPresenter(array('firstname' => 'First', 'lastname' => 'Last'));

isset($presenter['firstname']); // true
isset($presenter['lastname']);  // true
isset($presenter['fullname']);  // true
isset($presenter['notset']);    // false

Hopefully this is something that you'll find useful. I've updated the unit tests to cover the changes.

[robclancy]

Should just use method_exists in this because we don't want to be calling methods in an isset, that isn't what someone would expect to be happening and if their presenter had something like a query (yes people do stupid things) they would have issues with this. It just feels wrong to be calling it.

Other than that this is a good PR. It needs to be on the develop branch though.

[BenConstable]

Thanks very much for taking a look at this.

I understand that calling methods in isset() may provide unexpected overhead, but I can't really see that that would cause issues. Laravel's Eloquent models do just that internally in order to check the result of mutators. Also, as presenters are intended to be lightweight objects that interact with an already-loaded underlying model, there shouldn't be any heavy logic or database interactions in them (although I again understand your point about devs doing this - if they do do that though they're not really using the library correctly).

If you really don't want this behaviour however, then I don't think just using method_exists is a useful alternative, better just to forget this functionality. Otherwise it could be confusing and cause unexpected behaviour. For example (sorry it's a bit contrived):

<?php

class MyPresenter extends \Robbo\Presenter\Presenter {

    public function presentFullName()
    {
        if ($this->first_name && $this->last_name) {
            return $this->first_name . ' ' . $this->last_name;
        }

        return null;
    }
}

$p = new Presenter($object);

// This would be true, which would be confusing

if (isset($p->full_name)) {
    echo $p->full_name;
} else {
    echo $p->first_name;
}

Sorry about the incorrect base branch - couldn't find any contribution notes so just went for master by default! If you do want this functionality I'll remake the pull against develop.

[robclancy]

I'm still not really seeing that use case as something you would do instead of just doing if ($p->full_name)?
For example when you have the first_name in your database and it isn't set it will come back as and empty string in your code so you would always just do if ($p->first_name) and not an isset on it.

I just don't like the idea of executing a method just to see if it exists or returns null.

Yeah contributing stuff should be written along with more details in the readme or even a proper set of documentation. You can just keep updating this PR and I will manually merge it into develop myself when it is ready.

[BenConstable]

It was difficult to describe with a contrived example like the one I supplied, but you may be handing your view variables off to a third-party rendering library that uses isset() for its internal logic (Laravel's form building code does this when using form model binding for pre-population), or you may want to perform some logic based on whether a value is really empty.

What do you think would be a good alternative to making the method calls? As I said, I think using method_exists doesn't really provide much use and could make the API confusing.

[robclancy]

I'm going to get someone else's thoughts on this because I can't really decide.

[BenConstable]

Ok, thanks. Will keep an eye on the thread!

[robclancy]

I'll put it in as is, but once I have time which might not be until the weekend.

[BenConstable]

Great, thanks very much. If you do decide you want some changes just let me know.

[robclancy]

This is now merged with develop.