Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [stellation-res] Audit: overly complicated expression

On Tue, Sep 03, 2002 at 11:22:38PM -0500, Florin Iucha wrote:
> org.eclipse.stellation.workspace.List#listVersions contains the
> following snippet:
> 
>   String branchName =
>       getArguments().size() > 0  ? getArguments().remove(0)
>       : getWorkspace().getProject() != null  ?  getWorkspace().getProject().getBranchName()
>       : null;
> 
>   if (branchName==null) branchName = "main";
> 
> Personally I find ?: expressions where the alternatives are something
> other than constants or literals hard to parse. When I scan code my eyes
> get "stuck" to that expression...
> 
> I think this should be reworked as:
> 
>    String branchName = "main";
>    if (getArguments().size() > 0)
>    {
>       branchName = getArguments().remove(0);
>    }
>    else if (getWorkspace().getProject() != null)
>    {
>       branchName = getWorkspace().getProject().getBranchName();
>    }
> 
> florin
> 

By the way, on reviewing the code it should be written

   String branchName =
       getArguments().size() > 0  ? getArguments().remove(0)
       : getWorkspace().getProject() != null  ?  getWorkspace().getProject().getBranchName()
       : "main";

which reads as 

	Use an argument if given, or else the workspace project branch name, or else "main".

The corresponding

    String branchName = "main";
    if (getArguments().size() > 0)
    {
       branchName = getArguments().remove(0);
    }
    else if (getWorkspace().getProject() != null)
    {
       branchName = getWorkspace().getProject().getBranchName();
    }

suggests the branch name should be "main" by default unless otherwise specified. 

I find the conditional expression closer in spirit to the language used in the guide;
indeed, that's why it is written this way. 

If one wants to use the second form, and reflect the order used in the guide, one
could write:

    String branchName = null;
    if (getArguments().size() > 0)
    {
       branchName = getArguments().remove(0);
    }
    else if (getWorkspace().getProject() != null)
    {
       branchName = getWorkspace().getProject().getBranchName();
    }
    else {
       branchName = "main";
    }

dave
---
Dave Shields, IBM Research, shields@xxxxxxxxxxxxxx. 


Back to the top