-
Improvement
-
Resolution: Fixed
-
Major
-
None
-
-
5.0.0 (analysis-model and warnings-ng)
Issue has a constructor with String values for fileName, packageName, message, description. Another constructor is required (same number of parameters) that uses a TreeString for these arguments.
In a second step, IssueBuilder should be refined to store the members fileName, packageName, message, description in a TreeString as well.
Issue has a constructor with String values for fileName, packageName, message, description. The constructor has to be changed to required (same number of parameters) that uses a TreeString for these arguments.
All normalization and transformation logic that is currently part of the constructor of Issue should be moved to IssueBuilder#build. The Issue constructor then needs to be changed as described above.
[JENKINS-56792] Change constructor in Issue (TreeString)
| Epic Link |
New:
|
| Assignee | Original: Ulli Hafner [ drulli ] |
| Description | Original: Issue has a constructor with {{String}} values for fileName, packageName, message, description. Another constructor is required (same number of parameters) that uses a {{TreeString}} for these arguments. |
New:
Issue has a constructor with {{String}} values for fileName, packageName, message, description. Another constructor is required (same number of parameters) that uses a {{TreeString}} for these arguments. In a second step, {{IssueBuilder}} should be refined to store the members fileName, packageName, message, description in a {{TreeString}} as well. |
| Priority | Original: Minor [ 4 ] | New: Major [ 3 ] |
| Assignee | New: Nils Engelbrecht [ nengelbrecht ] |
| Status | Original: Open [ 1 ] | New: In Progress [ 3 ] |
| Rank | New: Ranked higher |
Good catch! Actually the normalization needs to be done outside of Issue. Otherwise the caching effect of TreeString would not pay off. A TreeString actually is a kind of String cache that tries to minimize memory consumption. In order to build up that cache the TreeStringBuilder instance must be created once and reused for all issues (i.e. it is a kind of static field). This is currently broken as the builder is created for each instance.
My idea about this issue was to provide the first part of the required refactoring. But after thinking again I think it makes more sense to rework that part completely. If it is not too complex for you we can rephrase this issue in the following way:
all normalization and transformation logic that is currently part of the constructor of Issue should be moved to IssueBuilder#build. The Issue constructor then needs to be changed as described above. That means no new constructor.
Then an IssueBuilder holds a TreeStringBuilder for each of the 4 fields and performs the whole transformation logic (and the rest of the String normalization). So the Issue constructor basically does nothing, just assigns the fields. Note that null should be not allowed anymore for these parameters, this should be ensured by the IssueBuilder.
Does this sound reasonable? If the work around that gets to complex we can also drop the second issue... Changing IssueBuilder and Issue in such a way will break several test cases, but I think it should be easy to spot what to do in those tests.
I understand. Sounds good to me!
After reworking this for the fileName parameter about 8 test cases are failing.
Am I allowed to delete test cases such as IssueTest#testFileNameBackslashConversion (therefore of course I would create a new one in the IssueBuilderTest class) ?
Another question I got is how to cope with failure of the test case IssueTest#shouldReadIssueFromOldSerialization ?
I assume this is due to the type-change of fileName from String to TreeString.
Do the values of those parameters (now being TreeStrings) have to be 'normalized' within the Issue class as well?
If so, I would either have to implement the normalizeFeature within the TreeString.class or have to retrieve the String of e.g the fileName, normalize it and then create a new TreeString via TreeStringBuilder.
Or should the normalization happen before being passed to the Issue-Constructor ( within the IssueBuilder)?
Example for 'normalization':
Issue.java(l. 305): this.fileName = builder.intern(normalizeFileName(fileName));