LANG-1366 : Add Feature for No ClassName and MultiLine StringStyle #308
Conversation
- Added the feature as a Inner Class to ToStringStyle - Added JUnit
|
It would be great to add this new TSS. Thanks! |
There was a problem hiding this comment.
Minor comments, but no blockers. Two users commenting here, pull request looks good. I have no immediate use case that I can think of for this (except for maybe logging perhaps). But also have no objection. Any thoughts from others?
Thanks for the pull request @pckeyan !
Bruno
src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/lang3/builder/ToStringStyle.java
Outdated
Show resolved
Hide resolved
1) Revised version to 3.10 2) Removed json only comment 3) Updated the Return type to be consistent with other classes 4) Updated the Unit Test
| /** | ||
| * Unit tests {@link org.apache.commons.lang3.builder.NoClassNameMultiLineToStringStyleTest}. | ||
| */ | ||
| public class NoClassNameMultiLineToStringStyleTest { |
There was a problem hiding this comment.
Hi @Stzx
Unfortunately I haven't had much experience with JUnit 5 yet. Looks to me like it will support multiple modifiers. But I couldn't find a recommendation, or even any deprecation, that would require dropping the public modifier. Do you know if that's a best practice, recommendation, or if it will be deprecated/required later?
I found at least one file in their repo using the public modifier: https://github.com/junit-team/junit5/blob/f52ded2d2f331ca7dba738b53afa5b1c0f22a85d/platform-tests/src/jmh/java/org/junit/jupiter/jmh/AssertionBenchmarks.java
There was a problem hiding this comment.
Found one comment that appears to be from someone involved with the project: https://stackoverflow.com/a/55230350
Doesn't seem to be required to use private, public, etc. It's up to the project.
There was a problem hiding this comment.
From the links you provided, as well as what I saw from the official examples (junit5-jupiter-starter-maven) and user guide seems to be determined by the project?
Looks different styles of different projects.
There was a problem hiding this comment.
Yeah, and if it's not something that can be checked with checkstyle or with the other build tools, we have no clear way to enforce... and having to ask users to alter their PR's can be a bit frustrating for the contributors.
For this reason I think it might be easier to just keep things as they are, and accept new submissions using either public, private, or package-protected.
|
TBH (and sorry for coming in a bit late), I am against a PR like this one. My concern here is that there is a huge number of configuration combinations possible for I'd hope not to see this class grow and grow and grow overtime; but, if this changes comes in, it becomes harder to say no to other niche configuration combinations. This specific PR is so, well, specific, that while I am sure the author feels it is general purpose enough for inclusion here, I would claim it is not and that it belongs in their application, not Commons Lang. |
|
@garydgregory I don't have a use case for this, so can't argue with your review. @pckeyan if you could elaborate more on your use case, or maybe others could chime in, then maybe your pull request could still be merged. Otherwise, this is probably something that will have to be implemented on your applications as a small utility class 👍 |
|
FWIW, I have a bunch of these application specific ToStringStyles in various projects but I do not think it appropriate to add these here. |
|
My usecase is to print multiline without classname. Usage I had was in MapReduce application. I thought it would be a contribution to reuse as there is no available option without classname. |
Added sub class to print with No Class Name in Multiline.
Added Junit.