[GROOVY-5851] wasted work in "SimpleGroovyClassDocAssembler.extractName"

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 2.0.5
• Fix Version/s: 2.0.6, 2.1.0-beta-1
• Flags: Patch
• Assignee: Paul King
• Reporter: Adrian Nistor
• Created: 17/Dec/12 18:43
• Updated: 07/Apr/15 19:13
• Resolved: 20/Dec/12 17:48

Description

The problem appears in version 2.0.5 and in revision 4cf5263.. I
attached a three-line patch (patch.diff) that fixes it. I will also
post (in the "Comments" section) the URL for the github pull request.

In method "SimpleGroovyClassDocAssembler.extractName", the loop over
"importedClassesAndPackages" keeps overriding "typeName" with "name".
Therefore, only the last written value is visible out of the loop and
all the other writes and iterations are not necessary. The patch
iterates from the end of "importedClassesAndPackages" and breaks the
first time when "typeName" is set.

The above fix (in patch.diff) is certainly correct (it's easy to see
through code inspection), but I think we can have an even shorter
patch (one line, in patchShort.diff), described below. There is no
Groovy test that touches this code location, so I am not 100% sure
this second patch (patchShort.diff) is correct, though it should be.

patchShort.diff is based on the fact that the condition
"if (name.endsWith(slashName))" (which decides if "typeName" is set or
not) is true at most once in the loop over
"importedClassesAndPackages". Even if it is true more than one time,
the "name" value is still legal. Thus, the loop can just break
immediately after "typeName" is set.