From e3bfcbcd25b177b870ff4dbdbe0e49be3bb86947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 26 Oct 2019 00:33:57 +0200 Subject: [PATCH] Apply Style/ClassVars rubocop rule Class variables in Ruby are not the same as instance variables of a class. They're particularly tricky when it comes to inheritance and modules. In the case of the Measurable module, for example, using a class variable will make all classes including the Measurable module share the same value. However, that's not what we want; we want the variable to be different for every class. And that's accomplished using a class instance variable. Although in this case it would probably be better to remove the caching variable. I don't think these methods are called more than once during a request, and even if they did it's highly unlikely the would become a bottleneck. --- .rubocop.yml | 3 +++ app/models/concerns/measurable.rb | 4 ++-- app/models/organization.rb | 4 ++-- app/models/user.rb | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index a5575bf5e..881bd428e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -376,6 +376,9 @@ Style/BlockDelimiters: Style/ClassCheck: Enabled: true +Style/ClassVars: + Enabled: true + Style/Not: Enabled: true diff --git a/app/models/concerns/measurable.rb b/app/models/concerns/measurable.rb index affb113c2..84ed5055d 100644 --- a/app/models/concerns/measurable.rb +++ b/app/models/concerns/measurable.rb @@ -3,11 +3,11 @@ module Measurable class_methods do def title_max_length - @@title_max_length ||= (columns.find { |c| c.name == "title" }.limit rescue nil) || 80 + @title_max_length ||= (columns.find { |c| c.name == "title" }.limit rescue nil) || 80 end def responsible_name_max_length - @@responsible_name_max_length ||= (columns.find { |c| c.name == "responsible_name" }.limit rescue nil) || 60 + @responsible_name_max_length ||= (columns.find { |c| c.name == "responsible_name" }.limit rescue nil) || 60 end def question_max_length diff --git a/app/models/organization.rb b/app/models/organization.rb index 32d545249..abda49873 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -42,11 +42,11 @@ class Organization < ApplicationRecord end def self.name_max_length - @@name_max_length ||= columns.find { |c| c.name == "name" }.limit || 60 + @name_max_length ||= columns.find { |c| c.name == "name" }.limit || 60 end def self.responsible_name_max_length - @@responsible_name_max_length ||= columns.find { |c| c.name == "responsible_name" }.limit || 60 + @responsible_name_max_length ||= columns.find { |c| c.name == "responsible_name" }.limit || 60 end private diff --git a/app/models/user.rb b/app/models/user.rb index 43927fe39..ee618566d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -313,7 +313,7 @@ class User < ApplicationRecord end def self.username_max_length - @@username_max_length ||= columns.find { |c| c.name == "username" }.limit || 60 + @username_max_length ||= columns.find { |c| c.name == "username" }.limit || 60 end def self.minimum_required_age