Thanks for the feedback @mattwynne!

Yeah. I agree with the `any?` call. I will commit and push a change shortly.

I think I will pull out the regex into a constant and put some
commentary around the various capture groups. The mapping of filter
expressions into a hash is a little complicated and probably needs to
be explained better with some focused unit tests and can be
restructured to be clearer.

Is it worth pulling the whole tag filter class into its own file and
adding some focused RSpec around it? It may help to document what it
is doing specifically.

I focused on getting this working first so I could get some feedback
sooner rather than later. Please let me know of any other smells/pain
points and I will get them fixed up as best I can.

On Fri, Jan 17, 2014 at 10:09 AM, Matt Wynne <notifications@github.com> wrote:
> In lib/cucumber/core/test/filters.rb:
>
>> +          def count_for(tag_name)
>> +            tag_name_counts[tag_name]
>> +          end
>> +
>> +          def locations_for(tag_name)
>> +            tag_name_locations[tag_name]
>> +          end
>> +        end
>> +
>> +        class TagLimits
>> +          attr_reader :limit_list
>> +          private :limit_list
>> +          def initialize(filter_expressions)
>> +            @limit_list = Hash[
>> +              Array(filter_expressions).map{ |filter_expression|
>> +                if matchdata =
>> filter_expression.match(/^(?:~)?(\@[\w\d]+)\:(\d+)$/)
>
> I presume this is plagiarised from the old code. I think this would benefit
> from some extracting and renaming to make it a bit more readable - I find it
> quite hard to read as it is.
>
> —
> Reply to this email directly or view it on GitHub.