HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsModerate

Setting the default display name for my model

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thesettingdefaultnamefordisplaymodel

Problem

I have a method in my Ruby code that sets the default name for my model. Rubocop complains that Assignment Branch Condition Size is too high, 21.24/15. How can I improve this?

def set_default_display_name
   return unless display_name.blank?
   count = user.credentials.where(type: type).count
   if count == 0
     self.display_name = name
   elsif user.credentials.where(display_name: name).count == 0
     self.display_name = name
   else
     self.display_name = "#{name} #{count + 1}"
   end
end

Solution

The method is doing three things:

  • Determining whether or not to set display_name.



  • Determining the default display name.



  • Setting display_name to the default value.



Each of those adds to the Abc metric. The biggest contributor to the Abc metric is determining the default display name. To me, that also seems like the most logically separate. We can lower the Abc metric by extracting that responsibility to its own method.

def set_default_display_name
  self.display_name = default_display_name if display_name.blank?
end

def default_display_name
  credentials_of_type = user.credentials.where(type: type)
  return name if credentials_of_type.none? || user.credentials.where(display_name: name).none?
  "#{name} #{credentials_of_type.count + 1}"
end


Extracting default_display_name makes it slightly simpler to test the default display name logic. You could also use that logic elsewhere, e.g., a UI could show the current display name and ask if they want to reset it to the default display name.

At this point, I'd question whether set_default_display_name is necessary or if it could be inlined.

Another option, is to keep your code as is and modify your .rubocop.yml file to increase the threshold of the Abc metric, or disable it entirely. The Abc metric is all about code size, not complexity. Theoretically, bugs increase with code size. You'd have to determine if this low of a threshold makes it easier to mask bugs. (Based on an earlier thread, the answer is probably, "yes", since your original code seems to have a bug. I retained the functionality of your original code in this example.)

--- Addendum ---

@ramonrails makes a good point in the comments. I'm usually fine with return guard clauses at the top of ruby methods. However, I agree that having a return in the middle of this method is not ideal.

This answer was only addressing the ABC question. If this were my code, I'd extract the credentials logic completely from this "view" code.

Something like:

def default_display_name(type_credentials)
  type_credentials.any? ? "#{name} #{type_credentials.count + 1}" : name
end


We're far afield from the question at that point though.

Code Snippets

def set_default_display_name
  self.display_name = default_display_name if display_name.blank?
end

def default_display_name
  credentials_of_type = user.credentials.where(type: type)
  return name if credentials_of_type.none? || user.credentials.where(display_name: name).none?
  "#{name} #{credentials_of_type.count + 1}"
end
def default_display_name(type_credentials)
  type_credentials.any? ? "#{name} #{type_credentials.count + 1}" : name
end

Context

StackExchange Code Review Q#86405, answer score: 12

Revisions (0)

No revisions yet.