From ddd9c07d3bd2b3245775b09c4731386f980d26f7 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 14:42:42 +0200 Subject: [PATCH 1/8] Fasterer: Use &:symbol is faster Calling argumentless methods within blocks is slower than using symbol to proc. --- app/controllers/contexts_controller.rb | 2 +- app/controllers/projects_controller.rb | 2 +- app/models/todo.rb | 2 +- test/controllers/projects_controller_test.rb | 78 ++++++++++---------- test/models/context_test.rb | 16 ++-- test/models/project_test.rb | 2 +- 6 files changed, 51 insertions(+), 51 deletions(-) diff --git a/app/controllers/contexts_controller.rb b/app/controllers/contexts_controller.rb index a4528b2e..306f686a 100644 --- a/app/controllers/contexts_controller.rb +++ b/app/controllers/contexts_controller.rb @@ -130,7 +130,7 @@ class ContextsController < ApplicationController # actions in the context will also be deleted. def destroy # make sure the deleted recurrence patterns are removed from associated todos - @context.recurring_todos.each { |rt| rt.clear_todos_association } unless @context.recurring_todos.nil? + @context.recurring_todos.each(&:clear_todos_association) unless @context.recurring_todos.nil? @context.destroy respond_to do |format| diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 29f6a07d..6cc51a86 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -267,7 +267,7 @@ class ProjectsController < ApplicationController end def destroy - @project.recurring_todos.each {|rt| rt.remove_from_project!} + @project.recurring_todos.each(&:remove_from_project!) @project.destroy respond_to do |format| diff --git a/app/models/todo.rb b/app/models/todo.rb index 7842a7a6..241b4461 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -188,7 +188,7 @@ class Todo < ActiveRecord::Base def touch_predecessors self.touch - predecessors.each { |p| p.touch_predecessors } + predecessors.each(&:touch_predecessors) end def removed_predecessors diff --git a/test/controllers/projects_controller_test.rb b/test/controllers/projects_controller_test.rb index da4e196f..b8cd2f70 100644 --- a/test/controllers/projects_controller_test.rb +++ b/test/controllers/projects_controller_test.rb @@ -1,47 +1,47 @@ require 'test_helper' class ProjectsControllerTest < ActionController::TestCase - + def setup end - + def test_projects_list login_as :admin_user get :index end - + def test_show_exposes_deferred_todos p = projects(:timemachine) login_as :admin_user get :show, :id => p.to_param assert_not_nil assigns['deferred_todos'] assert_equal 1, assigns['deferred_todos'].size - + t = p.todos.not_completed[0] t.show_from = 1.days.from_now.utc t.save! - + get :show, :id => p.to_param assert_equal 2, assigns['deferred_todos'].size end - + def test_show_exposes_next_project_in_same_state login_as :admin_user get :show, :id => projects(:timemachine).to_param assert_equal(projects(:moremoney), assigns['next_project']) end - + def test_show_exposes_previous_project_in_same_state login_as :admin_user get :show, :id => projects(:moremoney).to_param assert_equal(projects(:timemachine), assigns['previous_project']) end - + def test_create_project_via_ajax_increments_number_of_projects login_as :other_user assert_ajax_create_increments_count 'My New Project' end - + def test_todo_state_is_project_hidden_after_hiding_project p = projects(:timemachine) todos = p.todos.active @@ -52,7 +52,7 @@ class ProjectsControllerTest < ActionController::TestCase end assert p.reload().hidden? end - + def test_not_done_counts_after_hiding_and_unhiding_project p = projects(:timemachine) todos = p.todos.active @@ -64,15 +64,15 @@ class ProjectsControllerTest < ActionController::TestCase end assert p.reload().active? end - + # RSS - + def test_rss_feed_content login_as(:admin_user) get :index, { :format => "rss" } assert_equal 'application/rss+xml', @response.content_type #puts @response.body - + assert_xml_select 'rss[version="2.0"]' do assert_select 'channel' do assert_select '>title', 'Tracks Projects' @@ -94,31 +94,31 @@ class ProjectsControllerTest < ActionController::TestCase end end end - + def test_rss_feed_not_accessible_to_anonymous_user_without_token login_as nil get :index, { :format => "rss" } assert_response 401 end - + def test_rss_feed_not_accessible_to_anonymous_user_with_invalid_token login_as nil get :index, { :format => "rss", :token => 'foo' } assert_response 401 end - + def test_rss_feed_accessible_to_anonymous_user_with_valid_token login_as nil get :index, { :format => "rss", :token => users(:admin_user).token } assert_response :ok end - + def test_atom_feed_content login_as :admin_user get :index, { :format => "atom" } assert_equal 'application/atom+xml', @response.content_type # puts @response.body - + assert_xml_select 'feed[xmlns="http://www.w3.org/2005/Atom"]' do assert_select '>title', 'Tracks Projects' assert_select '>subtitle', "Lists all the projects for #{users(:admin_user).display_name}" @@ -133,70 +133,70 @@ class ProjectsControllerTest < ActionController::TestCase end end end - + def test_atom_feed_not_accessible_to_anonymous_user_without_token login_as nil get :index, { :format => "atom" } assert_response 401 end - + def test_atom_feed_not_accessible_to_anonymous_user_with_invalid_token login_as nil get :index, { :format => "atom", :token => 'foo' } assert_response 401 end - + def test_atom_feed_accessible_to_anonymous_user_with_valid_token login_as nil get :index, { :format => "atom", :token => users(:admin_user).token } assert_response :ok end - + def test_text_feed_content login_as :admin_user get :index, { :format => "txt" } assert_equal 'text/plain', @response.content_type assert !(/ /.match(@response.body)) end - + def test_text_feed_content_for_projects_with_no_actions login_as :admin_user p = projects(:timemachine) - p.todos.each { |t| t.destroy } - + p.todos.each(&:destroy) + get :index, { :format => "txt", :only_active_with_no_next_actions => true } assert (/^\s*BUILD A WORKING TIME MACHINE\s+0 actions. Project is active.\s*$/.match(@response.body)) assert !(/[1-9] actions/.match(@response.body)) end - + def test_text_feed_not_accessible_to_anonymous_user_without_token login_as nil get :index, { :format => "txt" } assert_response 401 end - + def test_text_feed_not_accessible_to_anonymous_user_with_invalid_token login_as nil get :index, { :format => "txt", :token => 'foo' } assert_response 401 end - + def test_text_feed_accessible_to_anonymous_user_with_valid_token login_as nil get :index, { :format => "txt", :token => users(:admin_user).token } assert_response :ok end - + def test_actionize_sorts_active_projects_by_number_of_tasks login_as :admin_user u = users(:admin_user) post :actionize, :state => "active", :format => 'js' - + assert_equal 1, projects(:gardenclean).position assert_equal 2, projects(:moremoney).position assert_equal 3, projects(:timemachine).position end - + def test_alphabetize_sorts_active_projects_alphabetically login_as :admin_user u = users(:admin_user) @@ -205,13 +205,13 @@ class ProjectsControllerTest < ActionController::TestCase assert_equal 2, projects(:gardenclean).position assert_equal 3, projects(:moremoney).position end - + def test_alphabetize_assigns_state login_as :admin_user post :alphabetize, :state => "active", :format => 'js' assert_equal "active", assigns['state'] end - + def test_alphabetize_assigns_projects login_as :admin_user post :alphabetize, :state => "active", :format => 'js' @@ -223,12 +223,12 @@ class ProjectsControllerTest < ActionController::TestCase end # XML (REST API) - + def test_xml_content login_as(:admin_user) get :index, { :format => "xml" } assert_equal 'application/xml', @response.content_type - + assert_xml_select 'projects' do assert_select 'project', 3 do assert_select 'name', /.+/ @@ -236,23 +236,23 @@ class ProjectsControllerTest < ActionController::TestCase end end end - + def test_xml_not_accessible_to_anonymous_user_without_token login_as nil get :index, { :format => "xml" } assert_response 401 end - + def test_xml_not_accessible_to_anonymous_user_with_invalid_token login_as nil get :index, { :format => "xml", :token => 'foo' } assert_response 401 end - + def test_xml_not_accessible_to_anonymous_user_with_valid_token login_as nil get :index, { :format => "xml", :token => users(:admin_user).token } assert_response 401 end - + end diff --git a/test/models/context_test.rb b/test/models/context_test.rb index e699609e..92630c38 100644 --- a/test/models/context_test.rb +++ b/test/models/context_test.rb @@ -13,21 +13,21 @@ class ContextTest < ActiveSupport::TestCase # only check that acts_as_list is present in the model assert @agenda.respond_to?(:move_to_bottom) end - + def test_validate_presence_of_name @agenda.name = "" assert !@agenda.save assert_equal 1, @agenda.errors.count assert_equal "context must have a name", @agenda.errors[:name][0] end - + def test_validate_name_is_less_than_256 @agenda.name = generate_random_string(256) assert !@agenda.save assert_equal 1, @agenda.errors.count assert_equal "context name must be less than 256 characters", @agenda.errors[:name][0] end - + def test_validate_name_is_unique newcontext = Context.new newcontext.name = contexts(:agenda).name @@ -36,7 +36,7 @@ class ContextTest < ActiveSupport::TestCase assert_equal 1, newcontext.errors.count assert_equal "already exists", newcontext.errors[:name][0] end - + def test_delete_context_deletes_todos_within_it assert_equal 7, @agenda.todos.count agenda_todo_ids = @agenda.todos.collect{|t| t.id } @@ -45,11 +45,11 @@ class ContextTest < ActiveSupport::TestCase assert !Todo.exists?(todo_id) end end - + def test_to_param_returns_id assert_equal '1', @agenda.to_param end - + def test_title_reader_returns_name assert_equal @agenda.name, @agenda.title end @@ -79,14 +79,14 @@ class ContextTest < ActiveSupport::TestCase @agenda.close! end - @agenda.todos.active.each {|t| t.complete! } + @agenda.todos.active.each(&:complete!) @agenda.close! assert @agenda.closed? end def test_activating_closed_context # given a context @agenda that is closed - @agenda.todos.active.each {|t| t.complete! } + @agenda.todos.active.each(&:complete!) @agenda.close! assert @agenda.closed? diff --git a/test/models/project_test.rb b/test/models/project_test.rb index fc482973..c676f14b 100644 --- a/test/models/project_test.rb +++ b/test/models/project_test.rb @@ -227,7 +227,7 @@ class ProjectTest < ActiveSupport::TestCase assert !p.stalled?, "completed projects are not stalled" p.activate! - p.todos.each{|t| t.complete!} + p.todos.each(&:complete!) assert p.todos.reload.active.empty?, "project should not have active todos" assert p.todos.reload.deferred_or_blocked.empty?, "there should not be deferred or blocked todos" assert p.reload.stalled?, "project should be stalled" From 0d224a5fe97bd5de6d841adc3b7801f5197ea188 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 14:49:52 +0200 Subject: [PATCH 2/8] fasterer: hash#fetch with block is faster hash#fetch with block is faster than hash#fetch with second parameter --- app/controllers/stats_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index d559686a..7369d456 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -448,7 +448,7 @@ class StatsController < ApplicationController end def three_month_avg(set, i) - (set.fetch(i,0) + set.fetch(i+1,0) + set.fetch(i+2,0)) / 3.0 + (set.fetch(i){ 0 } + set.fetch(i+1){ 0 } + set.fetch(i+2){ 0 }) / 3.0 end def set_three_month_avg(set,upper_bound) From 9f81e1a5c37db8f6134c8ffd854bc0081663a752 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 14:52:13 +0200 Subject: [PATCH 3/8] fasterer: Use attr_reader for reading ivars. --- app/controllers/todos/todo_create_params_helper.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/controllers/todos/todo_create_params_helper.rb b/app/controllers/todos/todo_create_params_helper.rb index 9eae5a3d..721f9064 100644 --- a/app/controllers/todos/todo_create_params_helper.rb +++ b/app/controllers/todos/todo_create_params_helper.rb @@ -1,7 +1,7 @@ module Todos class TodoCreateParamsHelper - attr_reader :new_project_created, :new_context_created + attr_reader :new_project_created, :new_context_created, :attributes def initialize(params, user) set_params(params) @@ -46,10 +46,6 @@ module Todos end end - def attributes - @attributes - end - def show_from @attributes['show_from'] end From 5092b388feedce31b9f8d5886f4c51ad3ab76779 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 15:12:52 +0200 Subject: [PATCH 4/8] Fasterer: Parallel assignment is slower Parallel assignment is slower than sequential assignment. Only got the low hanging fruit. There are some functions that have multiple return values. Fixing this needs more refactoring. --- app/controllers/todos_controller.rb | 20 ++++++--- app/helpers/projects_helper.rb | 3 +- lib/login_system.rb | 68 ++++++++++++++++------------- lib/tracks/utils.rb | 19 ++++---- 4 files changed, 63 insertions(+), 47 deletions(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index e0869f03..dbfda8ab 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -40,18 +40,21 @@ class TodosController < ApplicationController cookies[:mobile_url]= { :value => request.fullpath, :secure => SITE_CONFIG['secure_cookies']} determine_down_count - render :action => 'index' + render :action => 'index'.freeze end format.text do # somehow passing Mime::TEXT using content_type to render does not work - headers['Content-Type']=Mime::TEXT.to_s + headers['Content-Type'.freeze]=Mime::TEXT.to_s render :content_type => Mime::TEXT end format.xml do @xml_todos = params[:limit_to_active_todos] ? @not_done_todos : @todos render :xml => @xml_todos.to_xml( *todo_xml_params ) end - format.any(:rss, :atom) { @feed_title, @feed_description = 'Tracks Actions', "Actions for #{current_user.display_name}" } + format.any(:rss, :atom) do + @feed_title = 'Tracks Actions'.freeze + @feed_description = "Actions for #{current_user.display_name}" + end format.ics end end @@ -156,7 +159,10 @@ class TodosController < ApplicationController p = Todos::TodoCreateParamsHelper.new(params, current_user) tag_list = p.tag_list - @not_done_todos, @build_todos, @todos, errors = [], [], [], [] + @not_done_todos = [] + @build_todos = [] + @todos = [] + errors = [] @predecessor = nil validates = true @@ -887,13 +893,15 @@ class TodosController < ApplicationController elsif params[:format].nil? # if no format is given, default to html # note that if url has ?format=m, we should not overwrite it here - request.format, params[:format] = :html, :html + request.format = :html + params[:format] = :html end end def set_format_for_tag_view(format) # tag name ends with .m, set format to :m en remove .m from name - request.format, params[:format] = format, format + request.format = format + params[:format] = format params[:name] = params[:name].chomp(".#{format.to_s}") end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 503583ff..f6463263 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -25,7 +25,8 @@ module ProjectsHelper end def project_next_prev_mobile - prev_project,next_project= "", "" + prev_project = "" + next_project = "" prev_project = content_tag(:li, link_to_project_mobile(@previous_project, "5", @previous_project.shortened_name), :class=>"prev") if @previous_project next_project = content_tag(:li, link_to_project_mobile(@next_project, "6", @next_project.shortened_name), :class=>"next") if @next_project return content_tag(:ul, "#{prev_project}#{next_project}".html_safe, :class=>"next-prev-project") diff --git a/lib/login_system.rb b/lib/login_system.rb index ef8e0164..f2f97f9d 100644 --- a/lib/login_system.rb +++ b/lib/login_system.rb @@ -9,7 +9,7 @@ module LoginSystem def prefs current_user.prefs unless current_user.nil? end - + # Logout the {#current_user} and redirect to login page # # @param [String] message notification to display @@ -25,9 +25,9 @@ module LoginSystem redirect_to_login end end - + protected - + # overwrite this if you want to restrict access to only a few actions # or if you want to check if the user has the correct rights # example: @@ -39,7 +39,7 @@ module LoginSystem def authorize?(user) true end - + # overwrite this method if you only want to protect certain actions of the controller # example: # @@ -54,7 +54,7 @@ module LoginSystem def protect?(action) true end - + # When called with before_filter :login_from_cookie will check for an :auth_token # cookie and log the user back in if appropriate def login_from_cookie @@ -69,7 +69,7 @@ module LoginSystem flash[:notice] = t('login.successful') end end - + def login_or_feed_token_required if ['rss', 'atom', 'txt', 'ics'].include?(params[:format]) if user = User.where(:token => params[:token]).first @@ -79,7 +79,7 @@ module LoginSystem end login_required end - + # login_required filter. add # # before_filter :login_required @@ -90,19 +90,19 @@ module LoginSystem # def authorize?(user) # def login_required - + if not protect?(action_name) return true end - + login_from_cookie if session['user_id'] and authorize?(get_current_user) return true end - - http_user, http_pass = get_basic_auth_data - if user = User.authenticate(http_user, http_pass) + + auth = get_basic_auth_data + if user = User.authenticate(auth[:user], auth[:pass]) session['user_id'] = user.id set_current_user(user) return true @@ -111,22 +111,22 @@ module LoginSystem # store current location so that we can # come back after the user logged in store_location unless params[:format] == 'js' - + # call overwriteable reaction to unauthorized access access_denied return false end - + def login_optional login_from_cookie - + if session['user_id'] and authorize?(get_current_user) return true end - - http_user, http_pass = get_basic_auth_data - if user = User.authenticate(http_user, http_pass) + + auth = get_basic_auth_data + if user = User.authenticate(auth[:user], auth[:pass]) session['user_id'] = user.id set_current_user(user) return true @@ -134,22 +134,22 @@ module LoginSystem return true end - + def logged_in? current_user != nil end - + def get_current_user if @user.nil? && session['user_id'] @user = User.find(session['user_id']) end @user end - + def set_current_user(user) @user = user end - + # overwrite if you want to have special behavior in case the user is not authorized # to access the current operation. # the default action is to redirect to the login screen @@ -179,28 +179,34 @@ module LoginSystem session['return-to'] = nil end end - + # HTTP Basic auth code adapted from Coda Hale's simple_http_auth plugin. Thanks, Coda! def get_basic_auth_data - + auth_locations = ['REDIRECT_REDIRECT_X_HTTP_AUTHORIZATION', 'REDIRECT_X_HTTP_AUTHORIZATION', 'X-HTTP_AUTHORIZATION', 'HTTP_AUTHORIZATION'] - + authdata = nil - for location in auth_locations + auth_locations.each do |location| if request.env.has_key?(location) authdata = request.env[location].to_s.split end end if authdata and authdata[0] == 'Basic' - user, pass = Base64.decode64(authdata[1]).split(':')[0..1] + data = Base64.decode64(authdata[1]).split(':')[0..1] + return { + user: data[0], + pass: data[1] + } else - user, pass = ['', ''] + return { + user: ''.freeze, + pass: ''.freeze + } end - return user, pass end - + def basic_auth_denied response.headers["WWW-Authenticate"] = "Basic realm=\"'Tracks Login Required'\"" render :text => t('login.unsuccessful'), :status => 401 @@ -216,4 +222,4 @@ private end end -end \ No newline at end of file +end diff --git a/lib/tracks/utils.rb b/lib/tracks/utils.rb index 09e842cd..42ebd9b8 100644 --- a/lib/tracks/utils.rb +++ b/lib/tracks/utils.rb @@ -4,13 +4,14 @@ module Tracks class Utils AUTO_LINK_MESSAGE_RE = %r{message://<[^>]+>} unless const_defined?(:AUTO_LINK_MESSAGE_RE) - + # Converts message:// links to href. This URL scheme is used on Mac OS X # to link to a mail message in Mail.app. def self.auto_link_message(text) text.gsub(AUTO_LINK_MESSAGE_RE) do href = $& - left, right = $`, $' + left = $` + right = $' # detect already linked URLs and URLs in the middle of a tag if left =~ /<[^>]+$/ && right =~ /^[^>]*>/ # do not change string; URL is alreay linked @@ -30,25 +31,25 @@ module Tracks config = Sanitize::Config.merge(Sanitize::Config::RELAXED, :protocols => { 'a' => {'href' => Sanitize::Config::RELAXED[:protocols]['a']['href'] + ['onenote', 'message']}} ) - + rendered = Sanitize.clean(rendered, config) return rendered.html_safe end - + def self.textile(text) RedCloth.new(text).to_html end - + def self.sanitize_filename(filename) filename.gsub(/[^0-9A-z.\-]/, '_') end private - + def self.helpers ActionController::Base.helpers end - + end - -end \ No newline at end of file + +end From 405ad5a79f88741b00340bc9e2cc5ef67a1f63a7 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 15:19:47 +0200 Subject: [PATCH 5/8] Fasterer: Enumerable#sort is slower Enumerable#sort is slower than Enumerable#sort_by --- app/models/stats/projects.rb | 2 +- app/models/user.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/stats/projects.rb b/app/models/stats/projects.rb index d4a1f73b..4efcb171 100644 --- a/app/models/stats/projects.rb +++ b/app/models/stats/projects.rb @@ -22,7 +22,7 @@ module Stats def find_top10_longest_running_projects projects = user.projects.order('created_at ASC') - projects.sort{|a,b| b.running_time <=> a.running_time}.take(10) + projects.sort_by{ |p| p.running_time }.take(10) end end diff --git a/app/models/user.rb b/app/models/user.rb index d693c0f1..a46be896 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -34,7 +34,7 @@ class User < ActiveRecord::Base } end def projects_in_state_by_position(state) - self.sort{ |a,b| a.position <=> b.position }.select{ |p| p.state == state } + self.sort_by{ |p| p.position }.select{ |p| p.state == state } end def next_from(project) self.offset_from(project, 1) From d3aa73f783af820f87c2b2f2a74fd2aa3e3e8b27 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 15:21:11 +0200 Subject: [PATCH 6/8] first select than sort sorting on a smaller collection is faster --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index a46be896..e51f7eb6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -34,7 +34,7 @@ class User < ActiveRecord::Base } end def projects_in_state_by_position(state) - self.sort_by{ |p| p.position }.select{ |p| p.state == state } + self.select{ |p| p.state == state }.sort_by{ |p| p.position } end def next_from(project) self.offset_from(project, 1) From bb006f98c1a384b9b3913e3ad3d9b46dbdfe54e4 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 15:24:35 +0200 Subject: [PATCH 7/8] Using tr is faster than gsub Using tr is faster than gsub when replacing a single character in a string with another single character. Also freeze constant strings --- app/models/tag.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/tag.rb b/app/models/tag.rb index acbc3281..5c54575f 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -3,8 +3,8 @@ class Tag < ActiveRecord::Base has_many :taggings has_many :taggable, :through => :taggings - DELIMITER = "," # Controls how to split and join tagnames from strings. You may need to change the validates_format_of parameters if you change this. - JOIN_DELIMITER = ", " + DELIMITER = ",".freeze # Controls how to split and join tagnames from strings. You may need to change the validates_format_of parameters if you change this. + JOIN_DELIMITER = ", ".freeze # If database speed becomes an issue, you could remove these validations and # rescue the ActiveRecord database constraint errors instead. @@ -17,11 +17,11 @@ class Tag < ActiveRecord::Base # allow tags to be renamed later, you might want to use the # before_save callback instead. def before_create - self.name = name.downcase.strip.squeeze(" ") + self.name = name.downcase.strip.squeeze(' '.freeze) end def label - @label ||= name.gsub(' ', '-') + @label ||= name.tr(' '.freeze, '-'.freeze) end def to_s From 0602634ae3eaee5a7e0eb2aab50a895b1df0caa8 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 19 Aug 2015 15:42:14 +0200 Subject: [PATCH 8/8] Fasterer: Use #cover? instead of #include? on ranges --- app/models/recurring_todos/abstract_recurrence_pattern.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/recurring_todos/abstract_recurrence_pattern.rb b/app/models/recurring_todos/abstract_recurrence_pattern.rb index fe07a0f9..b8ef6d45 100644 --- a/app/models/recurring_todos/abstract_recurrence_pattern.rb +++ b/app/models/recurring_todos/abstract_recurrence_pattern.rb @@ -188,8 +188,8 @@ module RecurringTodos # Example: get 3rd (x) wednesday (weekday) of december (month) 2014 (year) # 5th means last, so it will return the 4th if there is no 5th def get_xth_day_of_month(x, weekday, month, year) - raise "Weekday should be between 0 and 6 with 0=sunday. You supplied #{weekday}" unless (0..6).include?(weekday) - raise "x should be 1-4 for first-fourth or 5 for last. You supplied #{x}" unless (0..5).include?(x) + raise "Weekday should be between 0 and 6 with 0=sunday. You supplied #{weekday}" unless (0..6).cover?(weekday) + raise "x should be 1-4 for first-fourth or 5 for last. You supplied #{x}" unless (0..5).cover?(x) if x == 5 return find_last_day_x_of_month(weekday, month, year)