JacobNinja / exercism-analysis

2 stars 2 forks source link

Conditional in loop analyzer #12

Closed JacobNinja closed 9 years ago

JacobNinja commented 9 years ago

Suggested in #8

Description: Looks for use of any conditional inside of an enumerated block (any `Enumerable method that takes a block argument)

Analyzer class: Exercism::Analyzers::EnumerableCondition

Result type: :enumerable_condition

Feedback type: :enumerable_condition

JacobNinja commented 9 years ago

@kytrinyx

This is the Enumerable method set:

ENUMERABLE_METHODS = %w(
        all? any? chunk collect collect_concat count cycle detect drop_while each
        each_cons each_entry each_slice each_with_index each_with_object
        find find_all find_index flat_map grep group_by inject map max max_by
        min min_by minmax minmax_by none? one? partition reduce reject reverse_each
        select slice_after slice_before slice_when sort sort_by take_while zip
      ).to_set

Feedback:

  1. I'm not fond of the name LoopConditional. Maybe EnumerableCondition or something better?
  2. I can change the feedback type on a per method basis if are planning on having different nitpicks for this analyzer

Yay! This is going to be awesome. :beers:

kytrinyx commented 9 years ago

This is going to be EPIC :)

I like EnumerableCondition better than LoopConditional.

This raises an interesting question, though (to be fixed separately, if at all). Do we need to break this into both result and feedback types? What if we just have one list of result types, and then the robot can figure out the feedback type separately.

JacobNinja commented 9 years ago

@kytrinyx I'm not satisfied with the current result/feedback approach because it seems redundant for most cases. I think it's a good idea to fix it. Can you describe the data structure that you think would be best for consumption in Rikki's case? It should be simple to adapt the processor result to our needs.

I renamed this analyzer to EnumerableCondition and I think it's ready to go. The result and feedback type are the same. Is that desired or do you think feedback types should represent the type of enumerable method? (ex: [map, collect, flat_map] => :map, [select, reject, take_while] => :filter)

kytrinyx commented 9 years ago

do you think feedback types should represent the type of enumerable method? (ex: [map, collect, flat_map] => :map, [select, reject, take_while] => :filter)

No, I think simpler is better. Later if we find that we have collisions or something we can think up something more complicated.

For rikki-, I wonder if it might be simplest to just have a single code word that represents the rule that triggered. If we start getting an unmanageably large list we could group them into categories, but it seems like that might not even be necessary.

What do you think?