# Determining if a product is taxable or imported

This should tell me whether the product is taxable or imported. Name should indicate if the product is imported or certain keywords should tell that the product is non-taxable (chocolates, book, pills).

Could you please review the following class products?

class Product
NON_TAXABLE = [/chocolates/, /book/, /pills/]

def initialize(product_name)
@product_name = product_name
end

def is_taxable?
taxable = false
NON_TAXABLE.each { |x| taxable = x.match(@product_name) if taxable }
!taxable
end

def is_imported?
/imported/.match(@product_name)
end
end

-

Your is_taxable? function code does not seem to work. The issue is that taxable is never going to be true, so you'll never assign a new value. You want to write if !taxable instead. Your code becomes:

def is_taxable?
taxable = false
NON_TAXABLE.each { |x| taxable = x.match(@product_name) if !taxable }
!taxable
end


It's still possible to make it simpler using Ruby functions on arrays.

def is_taxable?
NON_TAXABLE.none? { |x| x.match(@product_name) }
end


This means: if my product is not among the tax-free products, then it is taxable.

-
I got confused many times by the "double not logic" (a taxable product is not is non taxable list) and by Ruby double not, but this should be a correct answer now. –  Quentin Pradet Feb 14 '13 at 8:46
you can drop !!, a successful match is a truish value. –  tokland Feb 14 '13 at 10:06
Yeah, but I would expect is_taxable? to return true or false, not a partial match or false. I don't know what's idiomatic in Ruby though. –  Quentin Pradet Feb 14 '13 at 10:20
It's definetely idiomatic to return only true/false for a method?. But I was talking about the !! in the block, Enumerable#none? will always return a boolean no matter what. –  tokland Feb 14 '13 at 10:21
Right, updated. –  Quentin Pradet Feb 14 '13 at 10:36

If you change the NON_TAXABLE constant from a bunch of regular expressions to one big regular expression to rule them all, like this:

NON_TAXABLE_RE = Regexp.union(%w(chocolates books pills))


Then the is_taxable? method (I prefer taxable?) would look like this:

def is_taxable?
not NON_TAXABLE_RE.match(@product_name) #I like the explicit 'not'
end

-