Re: [PATCH] Standardizing OM for throwing TorqueException

[John McNally <jm...@collab.net>]

I tried to apply your patch and it failed due to file changes.  Other
than for readability, this situation is another reason to use unidiff's
as patch can do a better job of adjusting for changes in the file as
there is more context in the patch.  You get a unidiff with the option

cvs diff -u

not sure how you would specify this in wincvs, but it should have a
command line mode, if you can't find the option, you can tell the
difference easily as -u produces diffs with +/- as opposed to >/<.  Use
the latest cvs and I will apply it right away.

john mcnally

Mathieu Frenette wrote:
> 
> What a daunting task, but I did it!
> 
> It's my first involvement in Open Source, CVS, and Torque, so please be
> indulgent. ;-) I did my best with this action item.  Please let me know if I
> did anything wrong, so I can improve it next time.
> 
> I used WinCVS to produce the diff, but I don't know if it's "unidiff"
> compliant, as you mentionned that "unidiff" was preferred.  Also, I left all
> file diffs in the same text file.  Let me know if this poses a problem.
> 
> In addition to the patch file, I included a zip containing all the files
> after modification, just in case you need them.
> 
> Here's my work report:
> 
> *********************************
> 
> * Modified the following files for using TorqueException, and fixed javadoc
> comments:
> 
>     Torque.java
>     BasePeer.java
>     SqlExpression.java
>     Criteria.java
> 
>     MapBuilder.vm
>     Object.vm
>     Peer.vm
> 
> * Decided to leave the IdGenerator interface unchanged for now, because
> changing it to throw TorqueException instead of Exception might break
> people's code that implement this interface.  However, it would not affect
> code that is _using_ this interface.  We should discuss how this will be
> addressed.  If we settle to change it in some way, I can take care of it,
> including Torque classes which implement it.
> 
> * Simplified try/finally clauses (but only when it was in the scope of a
> required modification) from this style:
> 
>     dbCon = null;
>     try
>     {
>       dbCon = Torque.getConnection(dbName);
>       doDelete(criteria, dbCon);
>     }
>     finally
>     {
>       if (dbCon == null)
>           Torque.releaseConnection(dbCon);
>     }
> 
> to this style:
> 
>     dbCon = Torque.getConnection(dbName);
>     try
>     {
>       doDelete(criteria, dbCon);
>     }
>     finally
>     {
>       Torque.releaseConnection(dbCon);
>     }
> 
> Please confirm with me that this doesn't affect the intended semantic.  For
> example, I assumed that if .getConnection() failed, then
> .releaseConnection() doesn't need to be called.
> 
> * Tried to avoid excessive exception wrapping.  For example, in cases where
> SQLException, DataSetException and TorqueException are thrown by a portion
> of code, I explicitly catch the two first individually, and let
> TorqueException fall through.  (I could have caught Exception and wrap it
> into TorqueException, but that would allow TorqueExceptions to be wrapped
> many levels deep, which we don't want).
> 
> * Used the following style of try/catch/finally in order to simplify
> handling of cases where the finally clause may throw exceptions to be
> caught.  This is the cleanest way I know of handling this kind of situation:
> 
>     try
>     {
>         try
>         {
>             // Do something which may throw SQLException
>         }
>         finally
>         {
>             // Clean-up which may throw SQLException
>         }
>     }
>     catch(SQLException e)
>     {
>         throw new TorqueException(e);
>     }
> 
> *********************************
> 
> Comments would be appreciated! :-)
> 
> Looking forward to hearing from you...
> 
> -- Mathieu
> 
>   ------------------------------------------------------------------------
>                                       Name: StdUsageOfTorqueException.patch
>    StdUsageOfTorqueException.patch    Type: unspecified type (application/octet-stream)
>                                   Encoding: quoted-printable
> 
>                        Name: PatchedFiles.zip
>    PatchedFiles.zip    Type: Zip Compressed Data (application/x-zip-compressed)
>                    Encoding: base64
> 
>   ------------------------------------------------------------------------
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>

--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>