Skip to content

Use sexp node size for issue mass, not flay score #84

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

Merged
merged 1 commit into from
Jan 26, 2016

Conversation

ABaldwinHunter
Copy link
Contributor

This change fixes a bug, and impacts both remediation points
calculation and the issue mass reported in UI.

Motivation: create parity with Classic, and present user more
meaningful mass number.

An important behavioral departure from Classic is that we are no longer penalizing
extra for identical (as opposed) to similar duplicated code: remediation points
reflect the amount of effort needed to repair an issue, and perfectly identical code
can actually be easier to fix than code that is similar.

For reference, flay score is:

Similar code
node_size * number_of_occurrences_of_duplicated_issue

Identical code
node_size * num_occurrences * num_occurrences

@codeclimate/review @pbrisbin

Step two of breaking apart #78

@ABaldwinHunter
Copy link
Contributor Author

In Classic, we used effective_mass, which was the node_size for similar code, and node_size * num_occurrences for identical, or essentially:

flay_score / num_occurrences

@pbrisbin
Copy link
Contributor

[F]lay score is:

Similar code
node_size * number_of_occurrences_of_duplicated_issue

Identical code
node_size * num_occurrences * num_occurrences

In Classic, we used ... node_size for similar code, and node_size * num_occurrences for identical, or essentially:

flay_score / num_occurrences

I'm having a little trouble matching your commit/PR description to what's going on.

If I'm reading the diff correctly, what's really going on is that we're going to use Sexp#mass instead of Flay::Item#mass (?). Additionally, Flay::Item#mass was the "Flay" equations you show and Sexp#mass is the new flay_score / num_occurrences equation -- is all that right?

@ABaldwinHunter
Copy link
Contributor Author

@pbrisbin Almost all right.

flay score is indeed Flay::Item#mass

 Sexp#mass * num_occurrences (similar code)
 Sexp#mass * num_occurrences **2 (identical code)

Duplication engine had previously used flay score for mass when reporting mass in UI, and doing remediation points calculations.

Classic: used flay_score / num_occurrences (effectively), or

Sexp#mass (similar code)
Sexp#mass * num_occurrences (identical code)

This update: straight up Sexp#mass always

Better matches Classic.

Additionally, stop penalizing extra for duplicated code that is identical,
as its remediation effort should be the same or less than that of similar code.
@ABaldwinHunter ABaldwinHunter force-pushed the abh-mass-node-size branch 2 times, most recently from b6ec46b to 13820c5 Compare January 26, 2016 20:56
@ABaldwinHunter
Copy link
Contributor Author

@pbrisbin ready for another look!

  • passed boolean attribute identical to Violation instead of derivative string check_name
  • clarified commit message. for easy ref:
Use Sexp#mass for issue mass, not Flay::Item#mass

Better matches Classic.

Additionally, stop penalizing extra for duplicated code that is identical,
as its remediation effort should be the same or less than that of similar code.

@pbrisbin
Copy link
Contributor

LGTM

ABaldwinHunter pushed a commit that referenced this pull request Jan 26, 2016
Use sexp node size for issue mass, not flay score
@ABaldwinHunter ABaldwinHunter merged commit 3d18d90 into master Jan 26, 2016
@ABaldwinHunter ABaldwinHunter deleted the abh-mass-node-size branch January 26, 2016 23:05
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.

2 participants