search

wmakley / tiny_serializer

Simple Ruby JSON Serialization DSL. Replaces active_model_serializers.
MIT License
53 stars 2 forks source link

Use blank instead of empty #4

Closed GuskiS closed 6 years ago

GuskiS commented 6 years ago

https://github.com/wmakley/simple_serializer/blob/master/lib/simple_serializer.rb#L81

Use .blank? instead of .empty? cause .empty? creates database query ModelName Exists.

See for more: https://gist.github.com/AmitPatel-BoTreeConsulting/bd47d07a303956ba8b56

I think having .blank? will be fine cause you are going to work with that array anyway.

wmakley commented 6 years ago

Thanks, I hadn't thought about collections too hard yet. Solved a bug I had noticed in my logs actually.

I'd like the collection handling to be more explicit, it seems out of line with my peeves with AMS to handle them automatically, but it saved me time when I was deleting AMS.

Edit: I'm actually not sure that .blank? solves the issue either. For now I'm just going to forego this premature "optimization" until I'm more certain of how I want to handle collections.

GuskiS commented 6 years ago

IIRC, blank? was purely array/enum method, while empty? is ActiveRecord(or ActiveSupport) magic, so using blank? on ActiveRecord collection firstly transforms it to array and then checks length.

Anyways, I really enjoy this gem. I have the same reason as you for not using AMS. I even created issue https://github.com/rails-api/active_model_serializers/issues/2053 but didn't get any useful info.

Thank you for creating this 👍

wmakley commented 6 years ago

Thanks! I realized I shouldn't even be calling any other methods on the collection, since my definition of a collection is something that responds to each and not each_pair.