Comments on DP_Fetch with invalid data

Gravy posted on Thursday, January 15, 2009

Hi,

I'm wondering what peoples comments are on what is the best way of handling the DP_Fetch where the data requested doesn't exist. For example, fetching a Project where the Guid ID field doesn't match in the database. Should I throw an exception or silently ignore it and return an empty object?

I'm guessing this is going to be a 'depends' answer. Any pitfalls either way?

Thanks guys.

Graham

RockfordLhotka replied on Thursday, January 15, 2009

There are a couple very extensive threads on this topic on the forum - hopefully you can search and find them (www.lhotka.net/search.aspx may be helpful).

It is a 'depends' answer - mostly it depends on your views about how it should work :)

JoeFallon1 replied on Thursday, January 15, 2009

This question has come up many times over the past 5 years.

It depends. <g>

5 years ago about half the forum users thought one way and half the other.

I went with the idea of returning a New object with default values. My code in DP_Fetch has a branch to check if the record exits, and if not it calls DataPortal_Create instead.

I think I am in the minority now though. I believe the recommended practice is for the DataPortal to throw an exception. Your UI should expect this behavior and so should have a Try Catch block to present the user with a nice message that the record does not exist.

I cannot modify 5 years of work to accomodate this behavior so I am sticking with my existing pattern.

Joe

 

 

maxal replied on Thursday, January 15, 2009

I would agree that this is the matter of personal preferences. However, couple of general comments about exceptions.

I would argue that exceptions should be used for what they are, exceptions. Every rule has an exception (here we have a pun :) but what I mean is that it depends on what are you intentions.

There are two options, here. One is you are going to use your Get (Fetch) method to check if the object exists in database. In that case it is normal behavior and I would not use exception. For one, it is not an exception anymore but documented behavior, for another exceptions are costly.

There is another approach. You are not going to use Get (Fetch) to check object existence, so if it is not found it is indeed an exception, as something is wrong. In this case I would use exception, probably.

Gravy replied on Friday, January 16, 2009

Thanks guys for your input, all comments are very helpful.

I have decided to go ahead with throwing an exception in the Fetch as it is not used for checking object existance. However, I now find myself with another dilema...

If in the DP_Fetch method I throw a custom RecordNotFoundException I find that by the time the client UI gets it, it is wrapped in a DataPortalException. I see I have a few options available to me and was wondering what other people thought:

My client code could catch(DataPortalException ex) and check the BusinessException property. The thing I'm not keen on here is that is doesn't seem a 'logical' exception to catch.

I could catch the DataPoralException in the factory Get method and rethrow the inner BusinessException. Is there an easier way of doing this instead of in every factory method?

Can anyone suggest a better way?

Thanks again for your help.

Graham

ajj3085 replied on Friday, January 16, 2009

I think Csla move favors the approach you decided to take.. since it calls MarkOld after a fetch.  You could add some code to then call MarkNew after a fetch though.

Anyway...

It might not seem logical to catch a DataPortalException.. but that's how it works.  Usually my code looks like this:

catch( DataPortalException ex ) {
    if ( ex.BusinessException is RecordNotFoundException ) {
      MessageBox.Show( "Couldn't find specified record" );
    }
    else {
        throw;  // I typically have global exception handlers setup to display a custom error message.
    }
}

maxal replied on Friday, January 16, 2009

Since you indicated that exceptions will be thrown in the rear case of some inconsistency in database, I assume it could happen in multi-user environment only, my preference would be to handle DataPortalException on very high level and save time going through every Get method. After all, there may be other DataPortalExceptions to handle.

JoeFallon1 replied on Friday, January 16, 2009

Gravy:

Thanks guys for your input, all comments are very helpful.

I have decided to go ahead with throwing an exception in the Fetch as it is not used for checking object existance. However, I now find myself with another dilema...

If in the DP_Fetch method I throw a custom RecordNotFoundException I find that by the time the client UI gets it, it is wrapped in a DataPortalException. I see I have a few options available to me and was wondering what other people thought:

My client code could catch(DataPortalException ex) and check the BusinessException property. The thing I'm not keen on here is that is doesn't seem a 'logical' exception to catch.

I could catch the DataPoralException in the factory Get method and rethrow the inner BusinessException. Is there an easier way of doing this instead of in every factory method?

Can anyone suggest a better way?

Thanks again for your help.

Graham

 

Well -  you can't have it both ways!

The trade off is that your UI *has* to be prepared to get an excpetion back on every fetch. So you *have* to catch it. And you know that 99.9% of the time it is going to be a DataPortalException so that should be first in your list. You can catch general exceptions after that (like network errors or something.).

If you returned a New BO instead then you wouldn't have to bother with excpetions but you would have to check if it was the BO you asked for or a new one.

So bottom line - neither solution is free of side effects and you just have to adopt a model and move forward with it consistently.

Joe

RockfordLhotka replied on Friday, January 16, 2009

I do think it is totally valid to do factory methods like this:

public static CustomerEdit GetObject(int it)
{
  try
  {
    return DataPortal.Fetch<CustomerEdit>(...);
  }
  catch (DataPortalException ex)
  {
    throw ex.BusinessException;
  }
}

The purpose behind DataPortalException is to provide extra information about the exception. It does this by indicating what data portal method failed, and by providing access to the business object as it was when the exception occurred (in the case of an insert/update/delete).

It may be the case that your UI doesn't need any of this extra information. It may be that your UI would be simplified if it only got the original failure. In that case, this approach provides that behavior.

And this is probably still OK for development, because if you encounter exceptions and can't figure out what's going on, you can always use a debugger breakpoint to get into the factory method so the developer can see the full DataPortalException (which includes the complete stack trace).

Personally I allow the DataPortalException to go to the UI, because I think that makes debugging and troubleshooting easier - primarily because the full stack trace is right there where the exception is handled - but I can see arguments either way.

rasupit replied on Saturday, January 17, 2009

While I understand Rocky's point of view on wrapping all exceptions thrown by business object's code inside dp methods within DataPortalException.  I feel that this makes catching csla bo exception become a little bit unnatural compare to catching other .net object exception. 

The consequence is that we need to add additional instruction (in method comment) on how to access the exception through DataPortalException so developer that not familiar with CSLA does not get trip by this when consuming csla objects.  This issue become more noticable when you have (we have) diverse group of developer and not all your BOs are CSLA based.

Similar to Rocky's solution; In our shop, we decided extract the actual exception thrown by DataPortalException when the source for this exception is not CSLA framework (our DP code).  So when the exception is not from CSLA Framework, we re-throw the actual exception.  If the exception is from CSLA framework we just simply rethrow DataPortalException.

All our factory methods looks similart to this.  We also do the same with Save method by overriding this method in our base class.

            try
            {
                return DataPortal.Fetch<MyBo>(new Criteria(myKey));
            }
            catch (DataPortalException ex)
            {
                if (ex.BusinessException != null)
                    throw ex.BusinessException;
                throw;
            }

We found that this approach makes csla business objects behave more natural therefore user does not have to be aware/deal with this gotcha.

BTW: I found that new CSLA does not always return bo exception on BusinessException. see here. Can anybody confirm with this?

Ricky

Gravy replied on Wednesday, January 21, 2009

I thank you all for your help.

Right now, I'm going to take the route of re-throwing any Business  exceptions in the factory/Save methods. As I'm fairly new to CSLA.NET then this is all a bit of a learning exercise :-(

Regards

Graham