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

Refactoring: 30 small improvements/fixes #2456

Merged
merged 3 commits into from
Mar 31, 2020

Conversation

carloscaldas
Copy link
Collaborator

@carloscaldas carloscaldas commented Mar 25, 2020

Changes applied per file:

Road:

  • Transformed instance variables into class variables: 1
  • Added suffix to avoid loose of precision: 1
  • Assigned variable never used: 1
  • Removed redundant initialization: 1

PhyssimCalcLinkSpeedStats:

  • Transformed static variables into constants: 3
  • Transformed mutable object into immutable: 1
  • Extracted method: 1
  • Transformed instance variable into final instance variables: 5
  • Removed redundant comments: 8
  • Removed premature declaration: 1
  • Transformed Boxed type into primitive type: 1

VehicleMilesTraveledAnalysis:

  • Transformed instance variables into class variables: 1
  • Added suffix to avoid loose of precision: 1
  • Transformed instance variable into final instance variables: 2
  • Avoid NullPointerException: 1
  • Removed redundant comments: 1

#Relevant changes:

Bug fix:

Previous:

LinkedList<Double> gap = getGap();
if (gap == null) {
    gap = new LinkedList<>();
} else {
    gap.clear();
}

After:

if (gap == null) {
    gap = new LinkedList<>();
} else {
    gap.clear();
}

Comments: Assignment to the reference gap only updates the local variable that is not used

Avoid NPE:

Before:

this.vehicleTypes = vehicleTypes;

After:

this.vehicleTypes = vehicleTypes == null
    ? Set$.MODULE$.empty()
    : vehicleTypes;

Comments: The usage can lead to NPE. Other possible improvement would be to use Java Set instead of Scala Set since is a java class and no special feature from Scala collection is needed


This change is Reviewable

@carloscaldas carloscaldas self-assigned this Mar 25, 2020
@carloscaldas carloscaldas requested a review from REASY March 25, 2020 22:05
Copy link
Collaborator

@JustinPihony JustinPihony left a comment

Choose a reason for hiding this comment

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

Minor nit - otherwise LGTM

Copy link
Collaborator

@rajnikantsh rajnikantsh left a comment

Choose a reason for hiding this comment

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

LGTM

@carloscaldas carloscaldas merged commit 9bc107e into develop Mar 31, 2020
@carloscaldas carloscaldas deleted the ccaldas/refactor-2020-03-25-4ci branch March 31, 2020 13:38
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