Skip to content

Conversation

iggzq
Copy link

@iggzq iggzq commented Apr 11, 2025

In the ApplicationProperties.java, the lazyInitialization parameter explicitly declares its default value as false. However, the keepAlive, allowCircularReferences, and allowBeanDefinitionOverriding parameters do not have their default values of false explicitly declared, leading to an inconsistency in style.

Before Spring Boot 3.4, a similar inconsistency existed, where in the SpringApplication.java file, lazyInitialization was explicitly set to default to false, whereas keepAlive, allowCircularReferences, and allowBeanDefinitionOverriding did not have their default values of false explicitly stated.

In my opinion, explicitly declaring the default values for boolean objects makes them easier to understand.

…arameter explicitly declares its default value as false. However, the keepAlive, allowCircularReferences, and allowBeanDefinitionOverriding parameters do not have their default values of false explicitly declared, leading to an inconsistency in style. Before Spring Boot 3.4, a similar inconsistency existed, where in the SpringApplication.java file, lazyInitialization was explicitly set to default to false, whereas keepAlive, allowCircularReferences, and allowBeanDefinitionOverriding did not have their default values of false explicitly stated. In my opinion, explicitly declaring the default values for boolean objects makes them easier to understand.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 11, 2025
iggzq added 2 commits April 12, 2025 00:17
…arameter explicitly declares its default value as false. However, the keepAlive, allowCircularReferences, and allowBeanDefinitionOverriding parameters do not have their default values of false explicitly declared, leading to an inconsistency in style. Before Spring Boot 3.4, a similar inconsistency existed, where in the SpringApplication.java file, lazyInitialization was explicitly set to default to false, whereas keepAlive, allowCircularReferences, and allowBeanDefinitionOverriding did not have their default values of false explicitly stated. In my opinion, explicitly declaring the default values for boolean objects makes them easier to understand.

Signed-off-by: lituizi <2811328244@qq.com>
@nosan
Copy link
Contributor

nosan commented Apr 11, 2025

There are many similar inconsistencies throughout the codebase due to numerous people working on it. In my opinion, minor polishing changes like these don't significantly improve overall consistency - they only affect ApplicationProperties.java, while other classes remain untouched and likely contain the same or similar issues.

I am wondering if this could be addressed by using a Checkstyle rule such as ExplicitInitialization to enforce avoiding explicit initialization of primitives with their default values.

<module name="ExplicitInitialization">
	<property name="onlyObjectReferences" value="false"/>
	<property name="tokens" value="VARIABLE_DEF"/>
</module>
public class Example1 {
  private int intField1 = 0; // violation
  private int intField2 = 1;
  private int intField3;

  private char charField1 = '\0'; // violation
  private char charField2 = 'b';
  private char charField3;

  private boolean boolField1 = false; // violation
  private boolean boolField2 = true;
  private boolean boolField3;

  private Object objField1 = null; // violation
  private Object objField2 = new Object();
  private Object objField3;

  private int arrField1[] = null; // violation
  private int arrField2[] = new int[10];
  private int arrField3[];
}

@philwebb philwebb self-assigned this Apr 11, 2025
@philwebb
Copy link
Member

I'm not totally sure about a blanket rule, we do tend to set values in @ConfigurationProperties explicitly but generally not anywhere else. For now I've done a quick sweep of the code and removed the booleans that I think make sense. See 2143d70

@philwebb
Copy link
Member

I've opened #45168 for the checkstyle rule. Thanks for raising the PR @iggzq, but since we want to go the other way I'm going to decline this one in favor of commit 2143d70

@philwebb philwebb closed this Apr 11, 2025
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants