Changing count to size to prevent future DB query issues @group_notifications
@digitalmoksha @smcgivern @mdelaossa #34296 (closed)
As noted in #196251 (closed) we should avoid calling count
unless we really want it to recalculate using the DB.
But this case is special since @group_notifications
starts as a NotificationSetting::ActiveRecord_AssociationRelation
but after it's being appended a value it's converted to an Array
.
https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/profiles/notifications_controller.rb#L7 https://gitlab.com/gitlab-org/gitlab/blob/master/app/controllers/profiles/notifications_controller.rb#L8
But there are still a few issues with it.
-
Array#count
is still slightly slower thanArray#size
, as seen in https://github.com/JuanitoFatas/fast-ruby#arraylength-vs-arraysize-vs-arraycount-code . Also in the source code we can see slightly more action being performed forcount
before it results to the same action assize
.
https://apidock.com/ruby/Array/count https://apidock.com/ruby/Array/size
- If in the future someone will change
@group_notifications
and it will stay aaNotificationSetting::ActiveRecord_AssociationRelation
. And then we'll have an unneeded DB query.
Given that it currently keeps up logically equivalent but more "future resilient" I think we should change it to:
= _('Groups (%{count})') % { count: @group_notifications.size }