[
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.