What do your CRUD methods return?

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Re: What do your CRUD methods return?

Post by Weirdan »

VladSun wrote: Smells like a huge set of goto()-s :)
Goto's are not always evil :). Besides, you can easily catch exceptions inbetween the failure and global exception handler (you just don't have to), unlike goto's. Also, unlike goto's, exceptions bubble up the stack (in php you can't jump out of the function with goto).
VladSun wrote: Isn't it more appropriate to rethrow exceptions (more and more general maybe) while going up the chain?
It depends on the nature of the exception, because they could serve both as equivalent of 'return <error_indicator>' and die(). For example in application I'm working on now we're not handling some types of exceptions immediately but handle them on a global level instead (they are mainly access checks). If access check failed there's no sense to continue to process request, and there's almost nothing to be done (except, possibly, reverting db transaction and freeing some resources you might have acquired) before showing generic 'Oops, something went wrong. Please contact support.'.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: What do your CRUD methods return?

Post by josh »

matthijs wrote: $result = $model->insert($request);
// what should/could result be and what to do with it
If you don't have a tangible need for a return value, you shouldn't put in a return value just for the sake of having it there. Once I have a need for something, like to get the "last ID", I would create a method called getLastInsertId() and call that. I wouldn't try to "force feed" my method's caller a bunch of return values. Let the calling code "ask"!

Code: Select all

$model->insert($request);
$lastInsertId = $model->lastInsertId();
This has a secondary effect on overall quality because you aren't as tempted to couple the implementations of the insert & lastInsertId methods. They are clearly 2 different operations. One is a read one is a write.
Benjamin wrote:Exceptions are not the answer to everything, ... I enjoy testing conditions during assignment, such as in this example:
No offense but its a poorly constructed example because we were talking about arrays and your example uses scalars. My method could return an empty string and it would still be equal to false so I can't ascertain any deep understanding in your example. Maybe I can come up with something more compelling, consider this:

Example A - Always returns arrays, or would throw exceptions [that would bubble up] on error.

Code: Select all

$results = $model->listChildren(); // always an array
if( !count($results) ) // always an array
{
 echo 'no results';
}
foreach( $results as $result) // always an array
{
 echo $result;
}
vs

Example B - Returns false to signify 0 rows

Code: Select all

$results = $model->listChildren(); // is it an array or boolean?
if( !$results ) // lets check
{
 echo 'no results';
}
else
{
 foreach( $results as $result ) // if it was a boolean we have to "manually" skip this with an 'else' block
 {
 }
}
The more return types you need to deal with, the more nested your code. Every time you add another "return code", *every* caller of that method presumably has to be updated. Instead of adding return types and forcing the caller to deal with those return types, add methods and allow them to call them.
Simple typecasting can resolve the foreach issue.
If you need to have a return code for errors, OK. That is a legit decision to go non-OOP like you said, a decision to return false on error. But returning false "on success of finding 0 rows", and then type casting in the calling code, that would be circular logic. Solving the problem you created.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: What do your CRUD methods return?

Post by Benjamin »

josh wrote:No offense but its a poorly constructed example because we were talking about arrays and your example uses scalars.
The return type is not relevant, actually. You can return anything you like, and assume that if it evaluates to false, or IS false, then an error has occurred.

Again, an empty result set is different than an error condition, and so I feel it's important to be able to differentiate between the two.
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Re: What do your CRUD methods return?

Post by Weirdan »

Benjamin wrote:Sometimes it would I guess, but simple typecasting can resolve the foreach issue.
It can't if you're returning false:

Code: Select all

weirdan@virtual-debian: ~$ php -r 'var_dump(phpversion(), (array) false);'
string(7) "5.3.2-1"
array(1) {
  [0]=>
  bool(false)
}
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: What do your CRUD methods return?

Post by Benjamin »

Didn't throw any errors for me:

Code: Select all

foreach((array) false as $v) {
    
}
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Re: What do your CRUD methods return?

Post by Weirdan »

Benjamin wrote:Didn't throw any errors for me:
yeah, but you would not expect it to execute loop body even once, would you? But it did once, setting $v to false.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: What do your CRUD methods return?

Post by Benjamin »

I see what you mean. That's not something I ever do so I've never ran into an issue with that. I guess it boils down to preference.

This is a good example of how I write the controllers:

Code: Select all

    public function editDomainAction() {
        $il_domains = new il_domains();

        if (false == $dData = $il_domains->getDomainByIdAndUserId(il_request('id'), $this->uData->data->ID)) {
            $this->status->error('Sorry, the specified domain could not be found');
            $this->listDomainsAction();
            return;
        }

        if ($_SERVER['REQUEST_METHOD'] == 'POST') {
            if ($il_domains->updateDomainByIdAndUserId($_POST, il_request('id'), $this->uData->data->ID)) {
                $this->status->message('Success! Your changes have been saved.');
                $this->listDomainsAction();
                return;
            } else {
                $this->status->error($il_domains->getError());
            }
        } else {
            $_POST = $dData;
        }

        include IL_BASE_PATH . '/pages/domain_edit.php';
    }
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: What do your CRUD methods return?

Post by josh »

In the case of a finder, I return null objects, not scalars, in my applications. http://en.wikipedia.org/wiki/Null_Object_pattern

This way if the client says "everywhere a customer is not found we want to put this 'dummy' Name, or that 'dummy' photo", I have one place to encapsulate that, in the null object that subclasses a model class. If a model is not found, I get back a model that implements the same interface, but returns place holder data (for Example maybe the customer wants it to say "N/A" for the name, but "000-0000" for the phone number).

The null object helps to encapsulate those placeholder properties & behavior, and also keeps your methods down to 1 return type. Just because PHP allows us to have multiple return types doesn't mean its free on us effort wise. I have found null objects to be more effective.
It is not a pattern from Design Patterns, but is mentioned in Martin Fowler's Refactoring
So instead of checking if something is false, I can faithfully work with its interface polymorphic-ally. That was one of the "pillars" of this oop stuff after all.

When I need to break polymorphism, usually I'll have a PHP interface called Null_Object and make my null objects implement this interface. This interface will define no interface, but simply used for type checks in the event I need to ask an object if it is null. I could also look at its ID since its a model; but un-persisted models and null models are different concepts.

Code: Select all

if( $object instanceof Null_object) 
Of course polymorphism results in less nested code everytime. So avoid "checking" the object if you can, just work with it. Sometimes you'll need to check if its null and return out of a sub-routine early though.
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: What do your CRUD methods return?

Post by Benjamin »

Can you post a short example?
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: What do your CRUD methods return?

Post by josh »

Example Request to http://localhost/view?id=9999 <--------- and lets assume this refers to a non-existent model, no results found.

Controller:

Code: Select all

class Controller {
 function index($id) {
  $this->view->model = $this->find($id);
 }
 
 /** Just know it always returns a model, even if its just a null model */
 abstract function find();
}
 
View:

Code: Select all

<dd>Customer's Name</dd><dt><?=$this->escape($this->model->name())?></dt>
<dd>Customer's Phone</dd><dt><?=$this->escape($this->model->phone())?></dt>
Rendered Output

Code: Select all

Customers Name: N/a
Customers Phone: 000-0000
I could put an 'if statement' and skip the whole view rendering, but I don't have to is the point.
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: What do your CRUD methods return?

Post by Eran »

You're basically ignoring that no results were found. Usually for this case, we show a different screen than usual, something like "The item you requested was not found. If you think this is a mistake, please contact us" for a missing identifier, and for a list we show something like "There are no items at this time. Click here to add an item" or something of the sort. Your method might fail gracefully, but it's not great from a usability standpoint. To show a different screen, a conditional is needed regardless.

It's very rare that we want to show the same output regardless of whether items were found or not
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Re: What do your CRUD methods return?

Post by Benjamin »

Yeah, I mean I'm all about keeping things as simple as possible. I think it's important to be able to determine what happened and why.

How could you tell the difference between:

1. No results found
2. Search parameter error
3. System error
4. Unauthorized error

Further, how could you tell if that result came from the search or is the default value?
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: What do your CRUD methods return?

Post by josh »

pytrin wrote:You're basically ignoring that no results were found.
Kind of and kind of not. Like I said I could easily check if the object implements Object_Null and show said message. But consider if you had 20 results you pulled back, and a few of them were just missing odd fields here and there. Typically one would have "N/a" or placeholder photos. Rather than have if statements everywhere placeholder text could possibly go, a null object inverts the control, encapsulating that behavior within the model.
how would you tell the difference between:
1. No results found
If its a plural finder, it would be an empty array for iteration. $array == array(). If it were a singular finder it would return a null object for invoking methods on.
2. Search parameter error
? What do you mean? Like user asked for record 999 but there is no record 999? I'd get a null object and its ID would be 0. So
$model instanceof Object_Null && $model->id() == 0
3. System error
Those cause exceptions
4. Unauthorized error
My model would not be checking permissions. It would return the data, and the controller would decide what to do. Permissions are system level to me. Normally my controller has a utility method Like:

Code: Select all

$this->assertUserOwns( $model )
 
This would exit the controller & dispatch the error action if need be. Just a simple assertion.
Further, how could you tell if that result came from the search or is the default value?
If it implements Object_Null, or does not. Or if its id == 0 or not (although the latter could also be an un-persisted model)

Consider you had the following object hooked together

Code: Select all

Person
- Contact
--- Phone
--- Address
--- Email
- Photo
Now lets say the user has not uploaded a photo yet. I would have a regular Person object but it's ->photo() method would not return a regular Photo, it would return a Photo_Null which could either come from code generation or could be an actual sub-class I implemented. If you have to use an if statement everytime your program has conditional flow, you're doing it wrong IMO. Its much easier to read code that just outputs 100 fields. Its much harder to read the same code after its been peppered with control structures. Also if youre doing this "list" of users in more than one page, youre duplicating code ;-) (unless you use Null object pattern)

The reason you want to use these null objects is your photo may have a default filename, a default width, a default height, a default alt text, a default href, a default brightness, maybe cropping, maybe it in turn also composes some "Text" objects, etc..

Do you really want all that logic in your views, everytime you need to show a photo? Or would you rather just have a photo object and tell it to ->render() ?
User avatar
Eran
DevNet Master
Posts: 3549
Joined: Fri Jan 18, 2008 12:36 am
Location: Israel, ME

Re: What do your CRUD methods return?

Post by Eran »

You are mixing two concerns here - showing default values for some attributes for found items, and showing a somewhat different view for no found items. Those are not the same concerns. In the latter case, in my opinion it's important to have this logic in the view, as it's presentational logic and should indicate that we are showing something different than usual (such as a placeholder or a call-to-action).

Default values for missing attributes is handled separately and you normally couldn't tell just by the result of the finder method.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: What do your CRUD methods return?

Post by josh »

pytrin wrote:You are mixing two concerns here - showing default values for some attributes for found items, and showing a somewhat different view for no found items. Those are not the same concerns.
Whats an "attribute"? Whats an "item"? In your terms it would be an "item" (a Null_Object) which in turn encapsulate many default values. Maybe your default values arent static and can't be coded right in to the view script like you imply... maybe the default values need to be calculated. who knows. Having a null object doesnt prevent you from switching up the view though, just allows you to use the regular view seeded with default values, when you so chose.

If your default values constitute business logic, I would put it into the domain model. I in no way intended to say that all default values belong in the model. Only some. There is nothing wrong with putting all of them into the model though. Theres really no hard & fast rule. Ex. maybe you need a "translation" model that reads from language files? Or imagine an MMORPG where depending on the character class, a different default avatar is used. You certainly wouldn't want a "switch" statement right in the middle of your views, and overly breaking up your views causes an explosion of view scripts. If you have 10 properties with 10 possible default values, do you really need 10 conditionals all in the same file? or 100 view scripts? Its like the decorator pattern in that regard. It affords you flexibility, if you dont need it dont use it ;-)
Post Reply