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

Development: Refactor bad code smell in list to array conversion #5885

Merged
merged 7 commits into from
Nov 28, 2022

Conversation

MartinWitt
Copy link
Contributor

Repairing Code Style Issues

ToArrayCallWithZeroLengthArrayArgument

The performance of the empty array version is the same, and sometimes even better, compared
to the pre-sized version. Also, passing a pre-sized array is dangerous for a concurrent or
synchronized collection as a data race is possible between the size and toArray
calls. This may result in extra nulls at the end of the array if the collection was concurrently
shrunk during the operation.


See https://shipilev.net/blog/2016/arrays-wisdom-ancients/ for more details.

Repairing Code Style Issues

  • ToArrayCallWithZeroLengthArrayArgument (3)

- ToArrayCallWithZeroLengthArrayArgument
The performance of the empty array version is the same, and sometimes even better, compared
to the pre-sized version. Also, passing a pre-sized array is dangerous for a concurrent or
synchronized collection as a data race is possible between the <code>size</code> and <code>toArray</code>
calls. This may result in extra <code>null</code>s at the end of the array if the collection was concurrently
shrunk during the operation.</p>
See https://shipilev.net/blog/2016/arrays-wisdom-ancients/ for more details.
@MartinWitt MartinWitt requested a review from a team as a code owner November 25, 2022 17:55
@github-actions github-actions bot added the server Pull requests that update Java code. (Added Automatically!) label Nov 25, 2022
@artemis-bot artemis-bot added this to In progress in Artemis Development Nov 25, 2022
@Strohgelaender Strohgelaender changed the title refactor: refactor bad smell ToArrayCallWithZeroLengthArrayArgument Development: refactor bad smell ToArrayCallWithZeroLengthArrayArgument Nov 25, 2022
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

According to your own tool there are more instances of this code smell in the source code, e.g. in ConversationService.java:79. It would be great if you could adress all of them in one PR.

Artemis Development automation moved this from In progress to Review in progress Nov 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Base: 75.63% // Head: 79.92% // Increases project coverage by +4.28% 🎉

Coverage data is based on head (c3ebbb8) compared to base (91eae0b).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #5885      +/-   ##
=============================================
+ Coverage      75.63%   79.92%   +4.28%     
- Complexity         0    12354   +12354     
=============================================
  Files           1117     2125    +1008     
  Lines          44289    84225   +39936     
  Branches        8364    12633    +4269     
=============================================
+ Hits           33499    67313   +33814     
- Misses          5621     9385    +3764     
- Partials        5169     7527    +2358     
Flag Coverage Δ
client 75.63% <ø> (ø)
server 84.67% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...vice/connectors/bamboo/BambooBuildPlanService.java 83.01% <100.00%> (ø)
...ww1/artemis/service/metis/ConversationService.java 94.02% <100.00%> (ø)
...1/artemis/domain/enumeration/NotificationType.java 100.00% <0.00%> (ø)
...artemis/config/MessageConvertersConfiguration.java 100.00% <0.00%> (ø)
...emis/domain/quiz/DragAndDropQuestionStatistic.java 86.66% <0.00%> (ø)
...n/www1/artemis/web/rest/dto/OnlineResourceDTO.java 100.00% <0.00%> (ø)
...m/in/www1/artemis/service/ExerciseDateService.java 100.00% <0.00%> (ø)
...ww1/artemis/service/TutorParticipationService.java 64.65% <0.00%> (ø)
...emis/domain/metis/ReactionConstraintValidator.java 50.00% <0.00%> (ø)
...t/tutorialgroups/TutorialGroupSessionResource.java 83.20% <0.00%> (ø)
... and 1000 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

…nService.java

Co-authored-by: Lucas Welscher <micky.321@web.de>
Strohgelaender
Strohgelaender previously approved these changes Nov 26, 2022
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! 🎉

Strohgelaender
Strohgelaender previously approved these changes Nov 26, 2022
Copy link
Contributor

@JohannesStoehr JohannesStoehr left a comment

Choose a reason for hiding this comment

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

Code changes look good to me

Copy link
Contributor

@tobias-lippert tobias-lippert left a comment

Choose a reason for hiding this comment

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

Code LGTM. Nice code quality improvement.

@krusche krusche added this to the 6.0.0 milestone Nov 28, 2022
@krusche krusche changed the title Development: refactor bad smell ToArrayCallWithZeroLengthArrayArgument Development: Refactor bad code smell in list to array conversion Nov 28, 2022
@krusche krusche merged commit b27d53d into ls1intum:develop Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality ready to merge server Pull requests that update Java code. (Added Automatically!) small
Projects
No open projects
Artemis Development
  
Review in progress
Development

Successfully merging this pull request may close these issues.

None yet

6 participants