views:

331

answers:

9

Suppose I have a method

public Patient(int id) { ---- }

that returns Patient object given an id.. I could define contract in 2 ways

  1. Method would return null if patient does not exist
  2. Method would throw an exception if patient does not exist. In this case I would also define a query method that returns true if the Patient exist in the database or false otherwise...

Which contract should I use?? or any other suggestions

Update: Please comment on this case too... If it is not an database assigned Id and it is something a user enter in UI.. like SSN .. then which one is better..

Comment about Null pattern from Steve that I think is valid: probably not a good idea here, as it would be really useful to know immediately when an ID did not exist.

And I also think Null pattern here would be somewhat heavy weight

Comment from Rob Wells on throwing exception because its bad Id: i don't think a typo in a patient's name is an exceptional circumstance" IMHO

+3  A: 

Another option would be the Null Object pattern.

Chris Shaffer
probably not a good idea here, as it would be really useful to know immediately when an ID did not exist.
Steven A. Lowe
I'm not really seeing how that would be a benefit in this case.
Dalin Seivewright
Whether or not it is a benefit, he asked for other options; It *IS* a benefit to others who later read this question to know about this option.
Chris Shaffer
It is still possible to know immediately whan an ID does not exist, the line would be "if (LoadedUser == NullUser) {}" (or alternatively "if (LoadedUser.IsNullUser())" depending on how the class was written.
Chris Shaffer
+4  A: 

You should probably throw an exception. If you have an id that doesn't point to a valid patient, where did it come from? Something very bad has likely happened. It is an exceptional circumstance.

EDIT: If you're doing something other than an integer-based retrieval, like a search based on text, then returning null is fine. Especially since in that case you are returning a set of results, which could be more than one (more than one patient with the same name, same birth date, or whatever your criteria is).

A search function should have a different contract from a retrieval function.

Adam Bellaire
+1 - bad IDs are errors/exceptions
Steven A. Lowe
i don't think a typo in a patient's name is an exceptional circumstance" IMHO
Rob Wells
@[Rob Wells]: the function in question takes an integer ID as the input, not the patient's name
Steven A. Lowe
@[Steven A. Lowe]: Opps. Good catch.
Rob Wells
+1  A: 

In a simple situation like this 1. seems to be more than sufficient. You may want to implement something like a callback method that the client calls to know why it returned null. Just a suggestion.

Perpetualcoder
+2  A: 

For this circumstance, I would have the method return null for a non-existent patient.

I tend to prefer using exceptions to assist graeful degradation when there is a problem with the system itself.

In this instance, it is mosdt probably:

  1. a typo in the patient's ID if it was entered into a search form,
  2. a data entry error, or
  3. a workflow issue in that he patient's record hasn't been entered yet.

Hence, returning a null rather than an exception.

If there was a problem contacting the database, then I would have the method raise an exception.

Edit: Just saw that the patient ID in the signature was an integer, thanks Steven Lowe, so I've corrected my list of reasons.

My basic point about delineating when to use exceptions (for system errors) versus other methods of returning an error (for simple data entry typos) still stands though. IMHO.

HTH

cheers,

Rob

Rob Wells
+1  A: 

taking your descriptiong at face value, you probably need both:

  • bad IDs are errors/exceptions, as Adam pointed out, but
  • if you are given IDs elsewhere that might have disappeared, you will need the query method to check for them
Steven A. Lowe
A: 

Assuming I read that correctly... When you call Patient(100) it will return an object reference for a Patient with an id of 100. If no patient with an id of 100 exists, I think it should return null. Exceptions are overused IMO and this case doesn't call for it. The function simply returned a null. It didn't create some errored case that can crash your application (unless of course, you ended up not handling that null and passed it around to some other part of your application).

I would definitely have that function return 'null', especially if it was part of some search, where a user would search for a patient with a particular ID and if the object reference ended up being null, it would simply state that no patient with that id exists.

Dalin Seivewright
+2  A: 

It depends:

If you consider the normal operation will lead to a pation number not matching a file in the DB then an empty (NULL) record should be returned.

But if you expect that a given ID should always hit a record then when one is not found (which should be rare) then use an exception.

Other things like a DB connection error should generate an exception.
As you expect under normal situations the query to the DB to always work (though it may return 0 records or not).

P.S. I would not return a pointer. (Who owns the pointer??)
I would return an object that may or may not have the record. But that you can interogated for the existance of the record within. Potentially a smart pointer or somthing slightly smarter than a smart pointer that understands the cotext.

Martin York
+14  A: 

Keep in mind that going "over the wire" to another tier (whether a database or an application server) is one of the most expensive activities you can do - typically a network call will take several orders of magnitude longer than in-memory calls.

It's therefore worth while structuring your API to avoid redundant calls.

Consider, if your API is like this:

// Check to see if a given patient exists
public bool PatientExists(int id);

// Load the specified patient; throws exception if not found
public Patient GetPatient(int id);

Then you are likely to hit the database twice - or to be reliant on good caching to avoid this.

Another consideration is this: In some places your code may have a "known-good" id, in other places not. Each location requires a different policy on whether an exception should be thrown.

Here's a pattern that I've used to good effect in the past - have two methods:

// Load the specified patient; throws exception if not found
public Patient GetExistingPatient(int id);

// Search for the specified patient; returns null if not found
public Patient FindPatient(int id);

Clearly, GetExistingPatient() can be built by calling FindPatient().

This allows your calling code to get the appropriate behaviour, throwing an exception if something has gone wrong, and avoiding exception handling in cases where it is not needed.

Bevan
Nice though about alternative tiers. +1
Martin York
Awesome... Design by contract
StackUnderflow
A: 

Throw an exception.

If you return null, code like this:

Console.WriteLine(Patient(id).Name);

would fail with a NullReferenceException if the id doesn't exist, which is not as helpful as a say a PatientNotFoundException(id). In this example, it's still relatively easy to track down, but consider:

somePatient = Patient(id)

// much later, in a different function:

Console.WriteLine(somePatient);

About adding a function that checks whether a patient exists: Note this won't prevent PatientNotFoundExceptions completely. For example:

if (PatientExists(id))
    Console.WriteLine(Patient(id).Name);

-- another thread or another process could delete the patient between the calls to PatientExists and Patient. Also, this would mean two database queries instead of one. Usually, it's better to just try the call, and handle the exception.

Note that the situation is different for queries that return multiple values, e.g. as a list; here, it is appropriate to return an empty list if there are no matches.

oefe