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
AbcSize increase/investigation #8488
Comments
Maybe somebody will find this more readable: update script, which groups all line increments: https://gist.github.com/zverok/2f2a55c99f6093928103d58c1c149146 -- and its result: Outcome: sqrt(8 assignments² + 16 branches² + 8 conditions²) = 19.6
def setup_timecodes(interval)
# ^^^^^^^^ +assignment
words = paragraphs.flat_map(&:words)
# ^^^^^^^ +condition
# ^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +assignment
words.each { |w| w.timecodes = [] }
# ^^^^^^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^ +assignment
# ^ +assignment
# ^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +condition
return unless interval
# ^^^^^^^^^^^^^^^^^^^^^^ +condition
current = job.timecode || 0
# ^^^ +branch
# ^^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^ +condition
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +assignment
words.first.timecodes << current
# ^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +branch
words.each do |w|
# ^ +assignment
# ^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^ +condition
next if w.punct?
# ^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^ +condition
while current + interval <= w.time.to_i
# ^^^^^^ +branch
# ^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +condition (comparison)
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +condition
current += interval
# ^^^^^^^ +assignment
# ^^^^^^^^^^^^^^^^^^^ +assignment
w.timecodes << current
# ^^^^^^^^^^^ +branch
# ^^^^^^^^^^^^^^^^^^^^^^ +branch |
I'm sorry for the trouble. These Metrics cops had many issues. Hopefully they were all fixes at once, so as to create transition issues only once... Relevant changelog entries: 0.89.0
0.86.0
The PRs have additional discussions. Let me know if you have particular issues with some of the calculations; #8276 is an example of further tweak I made after some good feedback. |
@marcandre Oh, I didn't do my homework, it seems :) (I typically follow Rubocop development closely, but lately was on vacation and out of internetz... Should've checked changelogs more thoroughly.) Anyways, it was not an angry "you broke my stuff" ticket, rather curious "I wonder how it works" ticket. Your links make sense to me, I'll just raise thresholds a bit for now in our codebase (and probably will work more on my "explainer" script, I kinda love the idea). I am still a bit surprised that |
It's definitely debatable (and it was debated too 😆). There is no perfect measure, and Ruby in particular is troublesome as simple attribute readers need to be counted as "branches", while if Ruby could be compiled that could be inlined and not count. The threads give the reasoning for The main issue Goodhart's law. I know these revised metrics are more accurate and hopefully will fit your coding style even better than the previous one, albeit with increased defaults. It's just Metrics inflation 😅 |
After updating to 0.88 I noticed a lot of increases in CyclomaticComplexity in our codebase (= lots of new offenses); then, after 0.89, AbcSize and PerceivedComplexity grew drastically, too.
(I believe that it is not a good thing either way: after updating of large-ish codebase to Rubocop you suddenly find dozens of new offenses, and urged either to raise thresholds which felt previously "reasonable", or refactor a lot of code.)
In order to understand what exactly the numbers mean, and how to tackle complexity the best way, I wrote a draft of "expalnatory analyzer" script (ugly!): https://gist.github.com/zverok/5e858fac96a2e9f5282bbeab8fec2d39 -- which is just copy-pasted AbcSizeCalculator which adds meta-info about which node caused increase in one of A,B,C metrics. Now, the result for one method:
"Intuitively", it is "not good, not terrible" (YMMV), and Rubocop on our previous setting (AbcSize Max 17) was happy with it. After 0.89, Rubocop says 19.6. Here is the script's output with the investigation (note that line is repeated if several nodes of this line caused metric increase):
I am not 100% sure script output always points to "right" nodes, but I really hope so... Here, I can see several suspicious things (repeating the same output with additional comments):
I am not 100% sure ANY of the suspicious things shown above are some bugs, but they are... well, suspicious :) And the increase throughout the codebase bothers me, making me even more suspicious.
(Anyway, I believe that my "explainer" is a useful excercise that might one day grow into a gem. But I'll be happy if somebody could explain the weird increases)
The text was updated successfully, but these errors were encountered: