Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix JavaBeans naming convention #1023 #1030

Closed
wants to merge 2 commits into from

Conversation

hussachai
Copy link

Fix for issue #1023

Eclipse and Intellij generate getter and setter for pId as getpId and setpld respectively
while lombok generates getPld and setPId.
When the first character is lowercase and the second character is uppercase in this case 'I' ('I' as an Ice scream), the first character of the field name should not be capitalized.

@z-ji
Copy link

z-ji commented May 26, 2016

Yeah, I meet the same trouble with this, I consult the naming convention, and found many existing framework and IDEs like Spring Jackson Eclipse Intellij Netbeans use the naming convention of JavaBeans API specification which in the Chapter 8.8 "Capitalization of inferred names".I think the uniform naming convention is better, maybe Lombok think about this.

assertEquals("setaI", buildAccessorName("set", "aI"));
assertEquals("setaIr", buildAccessorName("set", "aIr"));

/* handle empty string cases */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a good practice to have a separate test for exceptions. Can you please take this try-catch section, and put it in a new test?

To verify if the correct exception was thrown or not, we state the expected Exception class in the @test annotation e.g
If we expect and ABCException to be thrown, we can specify it as

@test(expected = ABCException.class)

Further Reading: Test Annotation

assertEquals("setI", buildAccessorName("set", "I"));
assertEquals("setUrl", buildAccessorName("set", "Url"));

/* when the second character is uppercase, leave the first character unchanged */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also be shifted to a new method
testBuildAccessorNameWithUpperCaseSecondCharacterInSuffix()

Copy link

@armanSingh armanSingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes suggested to refactor the code.

@rzwitserloot
Copy link
Collaborator

As issue #757 explains, due to messups in the spec when javabeans were 'born', 'our' way is also a valid way. Slightly less valid but nevertheless:

  • 10% of the people want geteMail().
  • 6% of the people want getEMail().
  • 10% of the people don't care as long as we don't change it.
  • 74% of the people don't care no matter what we do, because they don't have any fields that have a single lower-case letter followed by an upper case later.

The math says 'leave it as is' wins, 16 to 10 (The current situation is getEMail).

Numbers made up, but unless you have pretty good proof I'm way off base with em I see no reason to doubt them.

@rzwitserloot
Copy link
Collaborator

NB: Assuming a lot more people complain about this, perhaps we can consider a lombok.config key so you can opt-in to 'geteMail' mode, but check with us first. As is, a pullreq with that change would not be accepted either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants