RESTfulPHP / controller / structure

Allow for stuff like:

• API/ExampleObjects

• API/ExampleObjects/SOME_INTEGER_ID/Children

• ...

with children knowing their owners (aka parents).

Full repo here

• Please be brutally honest if something can be improved!

• I couldn't add the rest tag as I don't have 150 rep yet.

```
0 ) {
// php 5.2 compatible
$pieces = explode('/', $params['request']);

if( is_numeric($pieces[0]) ) {
$this->setId( $pieces[0] );
$params['request'] = $request;
}
unset( $pieces );
} else {
// if the request is numeric, then it's an id
if( is_numeric($params['request']) ) {
$this->setId( $params['request'] );
$params['request'] = '';
}
}
}
}

public function getInstance( $params ) {
if( isset($params['owner']) && is_object( $params['owner'] ) ) {
$this->setOwner( $params['owner'] );
}

// if resource starts with "api/", strip it
if( strpos($params['request'], 'api/' ) === 0 ){
$params['request'] = substr($params['request'], 4);
}

if( $params['request'] ) {
if( strlen($request = trim(strstr($params['request'], '/'), '/')) > 0 ) {
// php 5.2 compatible
$pieces = explode('/', $params['request']);
$class = $pieces[0];
unset( $pieces );
} else {
$class = $params['request'];
}

$class = get_class($this) . $class;

$instance = new $class(array('request' => &$request));
$instance->setOwner( $this );

if( $request ) {
return $instance->getInstance(array(
'request' => $request
));
}

return $instance;
}
}

Good

• Private attributes for storing state in the class (id, owner).

• Getter and setter methods are simple.

Bad
Not a RestApi

• REST is an architectural style it can't be represented by an object. It is the wrong level of abstraction for an object to represent an architectural style.

• Your RestApi object has public methods: getInstance, getId and getOwner. These have nothing to do with representational state transfer.

Constructor

• The constructor should only create the object. By doing so much in your constructor you make it difficult to extend your class (going against the Open / Closed principle). It also makes it harder to unit test this code.

• If you construct your object with an array that doesn't contain the key 'request' your code will emit an error notice.

• unset($pieces); is unnecessary. Don't worry about memory management.

getInstance

• This method should not be called getInstance.

• This method does too much.

• It can call setOwner when it should only be getting an instance.

• Also, why should the class that you create depend on your RestApi object? get_class($this) will return 'RestApi' or 'RestApiDerived' if you derive another class from it. Why should the object that you create be so tightly coupled to the RestApi class?

I'm actually left confused by this class. The naming of methods and the object itself doesn't help me to understand what sort of objects this class represents. It doesn't map to anything in the real world that I can understand.

StackExchange Code Review Q#36049, answer score: 4