[DX] User::listing requires ugly logic to make it user-type agnostic

[kenloburn]

At the time of writing, to be able to ascertain the user ID requires making a specific call to the method of the type of the user you're expecting. If you would run the following piece of code:

$userId = User::listing($apiContext)[0]->getUserPerson()->getId();

You would run into an exception if the API HTTP response yields a business user (or vice versa when you use getUserCompany():

Fatal error: Uncaught Error: Call to a member function getId() on null in /Users/rukn01/Projects/rknol/sdk_php/example/monetary_account_example.php:20

So logically speaking if your integration is not aware of the kind of user that will come out of the API, the only way you can express this would be something like this pre-PHP 7.1:

$userWrapper = User::listing($apiContext)[0];
$userObj = !is_null($userWrapper->getUserPerson()) 
    ? $userWrapper->getUserPerson() 
    : $userWrapper->getUserCompany();

$userId = $userObj->getId();

Or PHP 7.1+ with the null-coalesce operator:

$userWrapper = User::listing($apiContext)[0];
$userId = ($userWrapper->getUserPerson() ?? $userWrapper->getUserCompany())->getId();

Either scenarios are unfavorable, and unnecessary if the design of the SDK is more developer friendly. I understand that the reasoning behind this is because resource-wise they're 3 different resources, however it makes for very ugly implementation.

One way to solve this would to abstract away the logic inside the User resource:

class User
{
    // ...
    public function getUserObject()
    {
        return !is_null($this->userPerson)
            ? $this->userPerson
            : $this->userCompany;
    }
}

Or something in the likes so that the DX will be:

$userId = User::listing($apiContext)[0]->getUserObject()->getId();

[dnl-blkv]

Good point, @kenloburn, we'll look into it!

[DennisSnijder]

@dnl-blkv this is pretty hard to do I suppose, since the API itself is designed to return different types of users.

[kenloburn]

Not necessarily..

[dnl-blkv]

@DennisSnijder fixing this would mean generating a wrapper. The task is quite straightforward, but does require some time. It is not hard, but not quick either :).

[OGKevin]

I have introduced a new method, which is called getReferencedObject and this is also present in User. https://github.com/bunq/sdk_php/blob/54d3754a767ba0959a8dee582ce0dac4054e535f/src/Model/Generated/Endpoint/User.php#L156

This will return the field that is not null as of both fields can not be set at the same time. The only improvement that is needed is to correctly cast the return value of this method. For now you would need to use an if statement that checks instanceof.

[OGKevin]

The above comment is the closest I can get this function via the generator. Im open for other suggestions either way.

The way #60 implemented it would require that every class that implements (anchor interface)[https://github.com/bunq/sdk_php/blob/652e650aa35a8c0c598653e986ac7f68d4808bdd/src/Model/Core/AnchorObjectInterface.php] to implement another interface with all the methods of all the classes the the getReferencedObject() method would return.

This will ofc not be possible or a vital solution as there are a lot of anchor implementations where its not the case that all the classes have the same methods. And therefore it would be hard to form a contract.

So, there is no nice way to fix this in the SDK with the generator.

Im open for other approaches and suggestions however. Closing for now.