Skip to content

Refactor getServletPath() and getPathInfo() for Conciseness#1103

Closed
noureldenashraf wants to merge 1 commit intovaadin:12.5from
noureldenashraf:12.5
Closed

Refactor getServletPath() and getPathInfo() for Conciseness#1103
noureldenashraf wants to merge 1 commit intovaadin:12.5from
noureldenashraf:12.5

Conversation

@noureldenashraf
Copy link

@noureldenashraf noureldenashraf commented Mar 1, 2025

#Description
Refactored getServletPath() and getPathInfo() methods to use the ternary operatorinstead of if-else. This change improves code conciseness while maintaining the same functionality.

#Type of change

  • Bugfix
  • [✔] Feature (Code Improvement)

#Checklist

  • [✔] I have read the contribution guide: [[Vaadin Contribution Guide]
  • [✔] I have added a description following the guideline.
  • [✔] The issue is created in the corresponding repository and I have referenced it.
  • [✔] I have added tests to ensure my change is effective and works as intended.
  • [✔] New and existing tests are passing locally with my change.
  • [✔] I have performed self-review and corrected misspellings.

#Additional for Feature type of change

  • [✔] Enhancement / new feature was discussed in a corresponding GitHub issue, and Acceptance Criteria were created.

#Rationale for Change

  • The ternary operator simplifies the code without altering its logic.
  • It eliminates unnecessary variable assignments while keeping the functionality intact.
  • The performance difference is negligible, and the new approach aligns with concise coding practices.

@CLAassistant
Copy link

CLAassistant commented Mar 1, 2025

CLA assistant check
All committers have signed the CLA.

@noureldenashraf noureldenashraf changed the title Optimized code by saving up memory Refactor getServletPath() and getPathInfo() for Conciseness Mar 1, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2025

@Legioth
Copy link
Member

Legioth commented Mar 1, 2025

Concise is often in conflict with debuggability (since it becomes more difficult to place a breakpoint or e.g. temporary logging in the right place) and readability. In this case, the exact context of the clarifying comments are lost which makes any benefits questionable.

@Legioth
Copy link
Member

Legioth commented Mar 1, 2025

I also notice the original description made a claim about saving memory. This is not true - the JIT compiler will treat both forms in exactly the same way. There's even a possibility that the suggested way will be slightly shower since it ends up calling the super method twice.

} else {
return super.getServletPath();
}
return (super.getPathInfo() == null) ? super.getServletPath() : "";
Copy link
Contributor

Choose a reason for hiding this comment

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

The proposed code seems to invert the original logic

Copy link
Member

Choose a reason for hiding this comment

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

Right. Yet another case for my claim that readability trumps conciseness.

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