Create stats class #15257
Create stats class #15257
Conversation
Moved lambda logic to own method -- #percentage_for_card
Breaks steps from #group_data into separate classes to reduce complexity of original method.
Added reminder to reduce number of initializer arguments.
ondenman
force-pushed
the
create-stats-class
branch
from
af47dbf to
fa2698d
Compare
| # frozen_string_literal: true | ||
| class TermStatistics | ||
| # TODO: reduce number of arguments | ||
| def initialize(term:, org_lookup:, people:) |
There was a problem hiding this comment.
I'm not sure what we want to do generally about documentation, but I think we definitely need to do something to explain what these arguments are. (And I strongly suspect that doing so would reveal org_lookup, at least, isn't really going to be a suitable thing to be passing around)
There was a problem hiding this comment.
Hmmm. After looking a little closer, I don't think people is obvious here either. I would naturally have assumed that that was going to be a list of EveryPolitician::Person objects, but nope!
|
I'm afraid I'm finding it really hard to puzzle out what this class represents. In particular, I can't easily tell what it gives me (and what I need to give it to get that). I suspect that 90% of this is a question of documentation, but I also suspect that writing that documentation will prove difficult enough to show that they might be wrong. (Adding unit tests for this may also show likewise…) |
Moving logic from term-table.rb to separate class.