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

matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

What do your CRUD methods return?

Post by matthijs »

If you have a model class with the common CRUD methods Create, Read, Update and Delete, what do you want those methods to return? There are different possibilities. For a read/find method, you could return false on error, an array on success. Or always return an array, empty or not. And in the controller code manually check for possible errors in the model.

For an update or delete method, you could return nothing. Or you could return the lastInsertId or rows affected.

For example, this is how I have my models now:

Code: Select all

class Articlemodel{
	public function find(){
		try {
			// .. query db
			return $result; // is an array
		} catch(){
		
		}
	}
	public function insert(){
		try {
			// .. query db
			return $result; // is the lastInsertId
		} catch(){
		
		}
	}
	public function update(){
		try {
			// .. query db
			return $result; // is the number of rows affected by the update
		} catch(){
		
		}
	}
	public function delete(){
		try {
			// .. query db
			return $result; // is the number of rows affected by the delete
		} catch(){
		
		}
	}
}

class ArticleController {
	public function insertArticle{
		$result = $model->insert();
		if($model->hasError()){
			// something went wrong, handle that
		} else {
			// do something with $result
		}
		
	}
}
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 generally write the controller first, so then I know what the requirements are for the model and what I need it to return.

I think for an insert, it's a good idea to return the insert id or false. For and update or delete, it's a good idea to return the affected rows or false on failure.
sam4free
Forum Newbie
Posts: 18
Joined: Fri Oct 10, 2008 6:02 pm

Re: What do your CRUD methods return?

Post by sam4free »

i don't think it's a good idea to return an array for the find() method .. the returned result may be too big ..
(you don't want to put a 10,000,000 search results in an array)
personally i use a special class to wrap the result pointer with methods like next(), prev(), ...
but i still have a getArray() method in that class in case i am sure -at the programming time- the result is small enough to fit in the memory all at once
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: What do your CRUD methods return?

Post by alex.barylski »

you don't want to put a 10,000,000 search results in an array)
On the contrary, using a native PHP array keeps your code more decoupled. Besides 99% of the time your queries should be returning paginated results of no more than 100 records at a time. The one exception to the rule is processing a newsletter mailing list, where you *might* iterate the recordset using the native RDBMS API.

Cheers,
Alex
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 »

For write operations, I return an array of errors on failure, primary key on insert and number of rows affected for update. For select operations, I return an array of rows on success and null on failure. Thinking about it, it might be better to return an empty array for that, I'm not sure.
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 »

I develop my controllers using TDD, which eliminates these types of questions. In TDD the API of your application comes a lot more naturally, and in my opinion is easier to change on a moments notice as well. The problem is just that, when you don't write a test first, you don't bother to think about it. Could you rationalize that to a client? "Hey client, I decided to write code without thinking about it first..." ;-) OR "we have to start over because I never wrote a spec and coded the wrong thing", OR "I spent so much time on XYZ feature we never got to ABC"

So instead of spending time even worrying about it, I would say write a test that uses the most obvious API. Then later you can just change the test to easily change the production code, should the need even arise. TDD stops you from writing programs that are 90% error handling and array shuffling, and no real "meat".

For example is returning "null" on "failure" really necessary? Couldn't an empty collection be returned instead, simplifying the calling code? Tests make the API easy, instead of making the implementation easy, which is what generally is going to matter.
Benjamin wrote:I generally write the controller first, so then I know what the requirements are for the model and what I need it to return.
That's *almost* TDD. You wrote the API before the implementation. Only difference is your examples have to be executed individually, one by one, by hand; during testing. Much better than implementing the code without any plan though!
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Re: What do your CRUD methods return?

Post by matthijs »

@josh: good point.

In the meantime I've decided:

Finder methods always return an array (or possibly a string, depending on the specific finder). So no return false or something. Error checking will be done separately.

Insert method returns lastInsertId.

Update/delete methods return affected rows.

I'm using try catch blocks and transactions for the db calls, so when something goes wrong with an insert/update or delete method, that should be caught by that.
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:For example is returning "null" on "failure" really necessary?
False rather, but Yes.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: What do your CRUD methods return?

Post by alex.barylski »

For example is returning "null" on "failure" really necessary? Couldn't an empty collection be returned instead, simplifying the calling code? Tests
I would agree yes, possibly. FALSE would indicate a failure, whereas an empty array would indicate there were no matching records. Depending on how you implement your software I suppose. I use Exceptions for all errors, but because of the single try-catch design of my applications, there are odd times when I need to manually test for failure.

For example, I had (at one point) a checkEmail() validation routine, which would throw exceptions on failure. This worked until I tried to use that same validation routine in an bulk import operation. When an email failed I needed to simply tets for failure, log it, and continue the operation as nothing happened, displaying a log at the end. With exceptions, this code was very difficult to re-enter and so Exceptions were dropped in favour of more traditional error handling.

Cheers,
Alex
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 »

If we're talking "failure" an exception is more robust than a return code. I'm NOT telling you which to use simply saying exceptions are more robust. That's a fact.

When no rows are found, returning an empty array results in a simpler API. One Return type is easier to test & deal with than Two return types. I'm not telling you there is an ideal number of return types, simply saying I could accomplish the same thing only 1 return type and I consider that a better solution. I'm saying I would have never even considered the 2 return types unless I was thinking from the "inside" of that method (its implementation) instead of the outside (its API).

Compare:

Code: Select all

foreach($array as $item)
{
 echo $item;
}
If the array is empty no harm is done, PHP skips the block.

To:

Code: Select all

if( !$array ) return false;
foreach( $array as $item )
{
 echo $item;
}
Same behavior as the first snippet, but more reading.

You see in the latter, the calling code also has to check the return type, and its calling code in return has to check that return type. Sooner or later 90% of your program is a big ball or "return mud", where as an exception can bubble up thru 1,000 layers of code without the programmer having to edit any of those "layers" seasoning them with return statements. Does that mean you should start having methods communicate via exceptions? No of course not. Methods should return data. Inputs & outputs, thats basic. My argument was that 1) true "errors" are better handled with exceptions. 2) An empty array is not an "error" type situation.
PCSpectra wrote:For example, I had (at one point) a checkEmail() validation routine, which would throw exceptions on failure. This worked until I tried to use that same validation routine in an bulk import operation. When an email failed I needed to simply tets for failure, log it, and continue the operation as nothing happened, displaying a log at the end. With exceptions, this code was very difficult to re-enter and so Exceptions were dropped in favour of more traditional error handling.
Exceptions are for exceptional situations. Databases going down, tables crashing, ethernet cords being pulled out of the server, RSS feeds going down, Queries failing, these are exceptional situations. Validating data is not an exceptional situation so you should use the return codes in that regard.

Guys basically we're discussing procedural code vs OOP in regards to error handling. "error codes" being the former, exceptions the latter. This likely stems from our PHP roots and PHP's late adoption of exception when languages like javascript have had them for years before us.

PS > This post is more of a "my 2 cents" thing. I'm not trying to telling Benjamin that the extra complexity wasnt warranted in his particular case, because obviously I dont know about his particular code. Im just saying that I consider one return type to be simpler than two, and that in the past tests have helped me find the lowest common denominators in my needs :drunk:
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: What do your CRUD methods return?

Post by alex.barylski »

I'm NOT telling you which to use simply saying exceptions are more robust. That's a fact.
No argument there, I was just saying, if your like me and a big proponent of consistency, then you tend to stick with one or the other and avoid using both techniques, until you encounter an issue like I did with email validaiton, at which point the traditional erro checking comes in handy.
When no rows are found, returning an empty array results in a simpler API. One Return type is easier to test & deal with than Two return types. I'm not telling you there is an ideal number of return types, simply saying I could accomplish the same thing only 1 return type and I consider that a better solution. I'm saying I would have never even considered the 2 return types unless I was thinking from the "inside" of that method (its implementation) instead of the outside (its API).
My email example doesn't apply here I suppose, but in the case of failure to match records, returning an empty array makes about as much sense as a FALSE but prevents multiple return types, so yes I would tend to agree.
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 »

PCSpectra wrote: failure to match records
You mean success of matching 0 records. Nothing failed. But yeah you got it. If it really failed, returning false is better than "silently" returning an array. An exception is even better than return false in an error scenario in my eyes for 2 reasons: 1)there is no return value at all during failures 2) the is only 1 valid return type during normal program flow. And again 0 rows is a success. Its only a failure once crap really hits the fan.

So I guess my relevant point is that exceptions scale better, for larger applications with huge call chains. For short call chains or simple programs it may be that returning an error code is more desirable than using try/catch blocks, but as you add layers & layers, it becomes impractical to pass a return value thru 100 call stack contexts.. it becomes a cross cutting concern in the code itself, which was [one] of the reasons we got exceptions. For when we need more robust error handling, and want to separate or uncouple the error handling from the normal program flow. In the same sense 3-tier isnt always better than 2-tier, return codes are no better/worse than exceptions for signifying errors. One just addresses needs that are different.


@matthijs
But I guess to answer the original question, you would have to define "crud methods". Are they controller methods? Methods on my data mapper? getters/setters on my model? Methods on an active record model? Methods on an active record class that has been de-coupled from its model? All this would determine the types of return values & exceptions of the respective component. SO in short you left out the "context" in your question.

Also empty catch blocks is a big sin. If I'm writing a "plugin" for your app you've basically ensured I get to deal with your nasty error suppression habits during my development time with your code. Not very nice of you ;-) I'd prefer you to catch exceptions of a specific type only, or leave the exception uncaught. Let it bubble up to "some one" who can deal with it better.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Re: What do your CRUD methods return?

Post by matthijs »

Great discussion.

@josh: by crud methods I mean the insert, update, delete and find methods of my models. Being used by the controllers. So:

Code: Select all

// controller
class ClimbController {
  public function addClimb {
    $model = $this->load()->model('ClimbsModel');
    if($request->isPost()){
      $result = $model->insert($request);
      // what should/could result be and what to do with it
  }
}
class ClimbsModel {
  public function insert($request){
     try {
        // .. query db
        return $result; // is the lastInsertId
     } catch(){
        // handle exception
     }
  }
}
So in this case the model::insert() returns the last inserted id on success and I think false on failure (I forgot exactly, have to check what PDO does there). On model::findSomething() it would in most cases return an array. As you made clear, it would be logical to always return an array, an empty one if there are no results.

About the empty catch blocks in my first example code: I just left that out for brevity. Just as I leave out more details. Maybe I should have mentioned that, but I thought it would be obvious
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Re: What do your CRUD methods return?

Post by VladSun »

//off topic
josh wrote:So I guess my relevant point is that exceptions scale better, for larger applications with huge call chains.
Smells like a huge set of goto()-s :)

Isn't it more appropriate to rethrow exceptions (more and more general maybe) while going up the chain?
There are 10 types of people in this world, those who understand binary and those who don't
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 think it's important for there to be both a distinction and a way to differentiate between an error condition vs no results found. An empty result set is different than an error such as invalid search parameters, unauthorized etc.

I certainly understand keeping code simple and I code with the same premise, but why would the code flow continue without results anyway? Sometimes it would I guess, but simple typecasting can resolve the foreach issue.

Exceptions are not the answer to everything, far more code is not OO, so when working with 3rd party codebases, you cannot just start throwing exceptions around without knowing where they will be caught.

I enjoy testing conditions during assignment, such as in this example:

Code: Select all

if (false == $result = $model->getXById($id)) {
    $message->addError($model->getError());
    $controller->someAction();
    return;
}
It's clearly not any more complex than previous examples posted in this thread, and at least to me it makes a lot of sense.
Post Reply