openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Dick (JIRA)" <j...@apache.org>
Subject [jira] Resolved: (OPENJPA-780) code review for DistributedStoreManager
Date Fri, 21 Nov 2008 20:49:44 GMT

     [ https://issues.apache.org/jira/browse/OPENJPA-780?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Michael Dick resolved OPENJPA-780.
----------------------------------

       Resolution: Fixed
    Fix Version/s: 2.0.0
                   1.3.0

Thanks very much for the patch Fernando!

> code review for DistributedStoreManager
> ---------------------------------------
>
>                 Key: OPENJPA-780
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-780
>             Project: OpenJPA
>          Issue Type: Improvement
>          Components: jdbc
>    Affects Versions: 1.2.0
>            Reporter: Fernando
>            Priority: Minor
>             Fix For: 1.3.0, 2.0.0
>
>
> I am currently reviewing code, and this one piece of code stood out.  It might not be
a bad thing, but it just has a "funny smell".  This is in DistributedStoreManager.  There
it gets a String[], which it then iterates over it to find an appropriate slice.  But it looks
like it either finds a slice and returns it, or finds a null and throws an exception, all
on the first step of the for loop.  So really, there is no for loop at all.  This might be
on purpose, but the code is just not as legible..
> ORIGINAL
>     /**
>      * Selects child StoreManager(s) where the given instance resides.
>      */
>     private StoreManager selectStore(OpenJPAStateManager sm, Object edata) {
>         String[] targets = findSliceNames(sm, edata);
>         for (String target : targets) {
>         	SliceStoreManager slice = lookup(target);
>         	if (slice == null)
>         		throw new InternalException(_loc.get("wrong-slice", target, sm));
>         	return slice;
>         }
>         return null;
>     }
> expecting more like:
> String[] targets = ....
> if ( targets == null || targets.length == 0 ) {
>     return null;
> }
> SliceStoreManager slice = lookup(targets[0]);
> if (slice == null) {
>     throw new InternalException(_loc.get("wrong-slice", target, sm));
> }
> return slice;

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message