Refactor to reduce number of loops

Only build the missing_keys array once we have detected that there
actually are missing keys by moving the check to be part of the block
that performs the path substitution.

1 files changed, 25 insertions, 15 deletions

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index fca663b2a3..0e5dc1fc6c 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -184,27 +184,18 @@ module ActionDispatch
             def optimized_helper(args)
               path = @string_route.dup
               klass = Journey::Router::Utils
-              parameterized_args = args.map(&:to_param)
-              missing_keys = []
 
-              parameterized_args.each_with_index do |arg, index|
-                if arg.nil? || arg.empty?
-                  missing_keys << @path_parts[index]
-                end
-              end
+              @path_parts.zip(args) do |part, arg|
+                parameterized_arg = arg.to_param
 
-              unless missing_keys.empty?
-                message = "No route matches #{Hash[@path_parts.zip(args)].inspect}"
-                message << " missing required keys: #{missing_keys.inspect}"
-
-                raise ActionController::UrlGenerationError, message
-              end
+                if parameterized_arg.nil? || parameterized_arg.empty?
+                  raise_generation_error(args)
+                end
 
-              @path_parts.zip(parameterized_args) do |part, arg|
                 # Replace each route parameter
                 # e.g. :id for regular parameter or *path for globbing
                 # with ruby string interpolation code
-                path.gsub!(/(\*|:)#{part}/, klass.escape_fragment(arg))
+                path.gsub!(/(\*|:)#{part}/, klass.escape_fragment(parameterized_arg))
               end
               path
             end
@@ -212,6 +203,25 @@ module ActionDispatch
             def optimize_routes_generation?(t)
               t.send(:optimize_routes_generation?)
             end
+
+            def raise_generation_error(args)
+              parts, missing_keys = [], []
+
+              @path_parts.zip(args) do |part, arg|
+                parameterized_arg = arg.to_param
+
+                if parameterized_arg.nil? || parameterized_arg.empty?
+                  missing_keys << part
+                end
+
+                parts << [part, arg]
+              end
+
+              message = "No route matches #{Hash[parts].inspect}"
+              message << " missing required keys: #{missing_keys.inspect}"
+
+              raise ActionController::UrlGenerationError, message
+            end
           end
 
           def initialize(route, options)