search

mdeering / attribute_normalizer

Adds the ability to normalize attributes cleanly with code blocks and predefined normalizers
MIT License
475 stars 53 forks source link

Use squeeze instead of gsub in SquishNormalizer #53

Closed samleb closed 10 years ago

samleb commented 10 years ago

According to these benchmarks, using squeeze is an order of magnitude faster than gsub when dealing with squishing/squeezing spaces on almost all Ruby implementations, except on Rubinius where it doesn't make much of a difference. Here is a patch that replaces gsub by squeeze in SquishNormalizer.normalize, with all tests green :)

mnaberez commented 10 years ago

ActiveSupport provides a String#squish method. Currently AttributeNormalizer has an implementation like that in Rails 3. The ActiveSupport implementation changed in Rails 4. I assume AttributeNormalizer does not just call String#squish to avoid a dependency on ActiveSupport.

I am not in favor of changing this normalizer to use squeeze(' ') because I think it would be surprising for Rails users to have a normalizer called squish that behaves quite differently from String#squish. I do agree that the specs for this normalizer should be improved.

samleb commented 10 years ago

Oh, sorry about that, I've never really came across this ActiveSupport String#squish method but I completely agree with you now that it'd be surprising indeed if the SquishNormalizer didn't behave like String#squish does. Anyway I think that the WhitespaceNormalizer does a perfect job as dealing with repeating spaces (and removing carriage returns too BTW), so I guess that was the behavior I was looking for in actual applications, it's just that as it wasn't in 1.1.0, I thought SquishNormalizer could do the job. Thanks again and sorry for the disturbance.