Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Refactor usage of magic getters in apis ? #301

Open
florianv opened this issue Dec 3, 2013 · 4 comments
Open

Refactor usage of magic getters in apis ? #301

florianv opened this issue Dec 3, 2013 · 4 comments

Comments

@florianv
Copy link
Contributor

florianv commented Dec 3, 2013

The magic getters inside a package can be handy but you can do this :

<?php
$github->issues->issues->issues....

which is not really nice to allow.

I think, ideally it should be structured this way

<?php

$issueManager = new IssueManager($options, $client);

// Finding an issue
$issue = $issueManager->findById(8);

// Creating an issue
$issue = new Issue();
$issue->setTitle('hello');
$issueManager->save($issue);

Each issue is a plain object with setters and getters and the manager acting as Data access object.
Ideally there should be an other layer so you can change the http client.

@dongilbert
Copy link
Contributor

I like that approach, it keeps separation of concerns, in that the object is not responsible for saving, as it would be in an ActiveRecord pattern. This would be something to consider for v2, as it would be a pretty big BC break.

@florianv
Copy link
Contributor Author

florianv commented Dec 3, 2013

Yes it would be an "Object Http Mapper"

@eddieajau
Copy link
Contributor

I think things like issues can be much simpler data objects. I don't see the need for getters and setters because we don't really have any control over the data structure. In other words, $issue->title = "Foo" is less overhead than $issue->setTitle('foo'). Then you could certainly do something like $issuesService->update($issue);.

@florianv
Copy link
Contributor Author

florianv commented Dec 3, 2013

Yes. Setters are only useful for typehinted params, but then it's a question of consistency, i.e if I use a setter once, maybe I should use them everywhere.

For example if we have somewhere in the package :

$issue->setRepo(Repository $repo) it would be odd to use later $issue->title = 'hello';

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants