Conversation
| return this; | ||
| } | ||
|
|
||
| private void regenerateEqualsAndHashCode(final JavaClassSource source) |
There was a problem hiding this comment.
Looks like this method should belong to the Refactory class?
e8e09d2 to
6d37208
Compare
|
Yes, I thought about that, too. Moved it to Refactory now. Additionally, the automatic detection of fields now skips non-final fields. The question is, should I stick with an automatic detection when any other method forces the user to specify the fields manually? |
|
That was my other thought. I think the optimal solution would be to make the method available for being called whenever the user requires it, passing the list of fields as arguments, wdyt? |
|
That way you would have to specify those fields every time you want to change the name or type of a field, unless the fields are stored with the class object. |
|
Perhaps we would need to introduce some selector methods, like Fields.all() or .nonFinal() to specify them in the method signature of Refactory.generateEqualsAndHashcode. Not really sure what would be the best strategy here |
| */ | ||
| public static void regenerateEqualsAndHashCode(final JavaClassSource source) | ||
| { | ||
| List<FieldSource<JavaClassSource>> finalFieldsToUse = new LinkedList<FieldSource<JavaClassSource>>(); |
There was a problem hiding this comment.
I think this can be parameterized
| List<FieldSource<JavaClassSource>> finalFieldsToUse = new LinkedList<FieldSource<JavaClassSource>>(); | ||
| for (PropertySource<JavaClassSource> property : source.getProperties()) | ||
| { | ||
| if (property.getField() != null && property.getField().isFinal()) |
There was a problem hiding this comment.
This doesn't check static fields, but I think we shouldn't assume anything. I like the idea of introducing field selectors or just passing the list of fields should be enough(the caller can use the streams API to filter accordingly)
|
The problem remains: How should the signature of |
|
I guess that the Refactory.regenerate method would have to be explicitly called by the user in this case. |
|
Yes, but that would contradict the initial intention of this issue, which sounds to me like an implicit action when So, what should I do? |
|
IMHO setType and setName should not change its current behavior. We can't assume that the equals/hashCode implementation would rely on final fields, for example. Also, It doesn't seem to be possible to find where such field is used (in a method body for example) so I'm ok with making clear that it is the caller's responsibility to call Refactory.createEquals/createHashcode whenever a change to a field is performed. Thus, I suggest we close the issue as Won't Fix and merge your code cleanup (which looks good) but don't introduce any different behavior. Sounds good? |
6d37208 to
197ca9a
Compare
|
I agree. Just performed a force-push of the changes, including a Javadoc comment for |
|
Thank you! You rock! |
No description provided.