Move reportable metric and cost fields to method

[lpichler]

@miq-bot add_label chargeback, refactoring

@miq-bot assign @gtanzillo

[miq-bot]

Checked commit lpichler@cd9dfc0 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
1 file checked, 0 offenses detected
Everything looks good. 🍰

[chrisarcand]

What benefits does this change provide?

This provides YACM (Yet Another Class Method™) that simply provides an even larger public interface to an otherwise private implementation (Class Method Spaghetti™), so unless it's removing duplication, has some sort of public API consideration...I see it as a more negative change than beneficial.

[isimluk]

I see it as a slightly positive change more than negative, as it reduces complexity of overly complex algorithm. The other benefit is that the chunk of code got somewhat useful name, so it can be better understood.

We have discussed this with @lpichler and I think he will be able to put this method into ChargebackRateDetail once we refactor defined_column_for_report condition out of it.

I think this became more obvious only after we started thinking about this chunk of code as a method. Finding a proper class to define it and perhaps changing scope is natural next step. :)

[chrisarcand]

as it reduces complexity of overly complex algorithm.

No complexity is saved here; it's simply moved and given another name.

Finding a proper class to define it and perhaps changing scope is natural next step. :)

Yup, a step toward something even better I can understand :) If this helps in future refactoring, sounds good. My main gripe is that it opens a public API that may or may not be appropriate for consumption by anything but the way it is now, so when possible I'd love to privatize whatever you extract. You can even use private_class_method. But as you're talking about moving this around soon, I won't block you further!

LGTM 👍

[lpichler]

thanks!

[chrisarcand]

Whoops, we're now in stabilization period for Euwe.

[lpichler]

marking back to euwe/yes because we have PRs (#11685, #11648 ) which depends on this.

@miq-bot remove_label euwe/no

[miq-bot]

@lpichler unrecognized command 'remove_add', ignoring...

Accepted commands are: add_label, remove_label, rm_label, assign, set_milestone

[lpichler]

@miq-bot add_label euwe/yes

[chessbyte]

Euwe Backport details:

$ git log -1
commit fb25610d7f6edd98d6deb0910044ccaa8458ec11
Author: Chris Arcand <chrisarcand@users.noreply.github.com>
Date:   Tue Oct 4 09:55:21 2016 -0500

    Merge pull request #11647 from lpichler/move_reportable_fields_to_method

    Move reportable metric and cost fields to method
    (cherry picked from commit cb340f9d83dc9480ec7b35725ec2ebcec0acaaf9)