| RE: [stellation-res] Audit: overly complicated expression |
On Wed, 2002-09-04 at 10:11, Jonathan Gossage wrote: > > > > >-----Original Message----- > > >From: stellation-res-admin@xxxxxxxxxxxxxxx > > >[mailto:stellation-res-admin@xxxxxxxxxxxxxxx]On Behalf Of Florin Iucha > > >Sent: September 4, 2002 12:23 AM > > >To: stellation > > >Subject: [stellation-res] Audit: overly complicated expression > > > > > > > > >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 > > > > > An alternative formatting using conditionals is: > > String branchName = > (getArguments().size() > 0) ? getArguments().remove(0) > : ((getWorkspace().getProject() != NULL) > ? > getWorkspace().getProject().getBranchName() > : null); > > The guidelines that enhance readibility and reliability are > a) Enclose the condifional in parentheses to avoid tripping over operator > precedence. > b) Align the two action clauses for each conditional under each other. > c) Enclose entire nested conditionals in parentheses to avoid possible > problems with operator precedence. > d) Indent nested conditionals. > > This is a good example, though, of how hard it is to find style idioms that > work well for everyone. Thus I find the fragment using conditionals easier > to read than the "if/else" style because I can see the nested structure more > clearly but others will undoubtedly prefer Florin's proposal. I have to say that I take Florin's side on this one... When I look at the nested ?:, even when formatted, I find it hard to read, except by translating it into the if/then in my head. Like Florin, I find ?: conditionals very difficult to handle in any but the simplest of cases. Personally, I'd rather not ever see a ?: nested in a ?:. -Mark -- Mark Craig Chu-Carroll, IBM T.J. Watson Research Center *** The Stellation project: Advanced SCM for Collaboration *** http://www.eclipse.org/stellation *** Work Email: mcc@xxxxxxxxxxxxxx ------- Personal Email: markcc@xxxxxxxxxxx
Attachment:
signature.asc
Description: This is a digitally signed message part