CR feedback

samples/RoutingSample/RouteCollectionExtensions.cs

@@ -11,7 +11,7 @@ namespace RoutingSample
         {
             if (routes.DefaultHandler == null)
             {
-                throw new InvalidOperationException("DefaultHandler must be set.");
+                throw new ArgumentException("DefaultHandler must be set.");
             }
 
             return AddPrefixRoute(routes, prefix, routes.DefaultHandler);

samples/RoutingSample/Startup.cs

@@ -3,7 +3,7 @@
 #if NET45
 
 using Microsoft.AspNet.Abstractions;
-using Microsoft.AspNet.Routing.Template;
+using Microsoft.AspNet.Routing;
 using Owin;
 
 namespace RoutingSample

src/Microsoft.AspNet.Routing/RouteCollectionExtensions.cs

@@ -2,8 +2,9 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.AspNet.Routing.Template;
 
-namespace Microsoft.AspNet.Routing.Template
+namespace Microsoft.AspNet.Routing
 {
     public static class RouteCollectionExtensions
     {
@@ -23,7 +24,7 @@ namespace Microsoft.AspNet.Routing.Template
         {
             if (routes.DefaultHandler == null)
             {
-                throw new InvalidOperationException("DefaultHandler must be set.");
+                throw new ArgumentException("DefaultHandler must be set.");
             }
 
             routes.Add(new TemplateRoute(routes.DefaultHandler, template, defaults));

src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Threading.Tasks;
 using Microsoft.AspNet.Abstractions;
 
@@ -76,20 +77,25 @@ namespace Microsoft.AspNet.Routing.Template
 
         public void BindPath(BindPathContext context)
         {
-            // This could be optimized more heavily - right now we try to do the full url
-            // generation before validating, but we could do it in two phases.
-            var path = _binder.Bind(_defaults, context.AmbientValues, context.Values);
-            if (path == null)
+            // Validate that the target can accept these values.
+            _target.BindPath(context);
+            if (!context.IsBound)
             {
                 return;
             }
 
-            context.BoundPath = path;
-            _target.BindPath(context);
-
-            if (!context.IsBound)
+            // This could be optimized more heavily - right now we try to do the full url
+            // generation after validating, but we could do it in two phases.
+            var path = _binder.Bind(_defaults, context.AmbientValues, context.Values);
+            if (path == null)
             {
-                context.BoundPath = null;
+                context.IsBound = false;
+                return;
+            }
+            else
+            {
+                Debug.Assert(context.IsBound);
+                context.BoundPath = path;
             }
         }
     }

test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTests.cs

@@ -76,7 +76,7 @@ namespace Microsoft.AspNet.Routing.Template.Tests
         }
 
         [Fact]
-        public async void Match_RejectedByHanlder()
+        public async void Match_RejectedByHandler()
         {
             // Arrange
             var route = CreateRoute("{controller}", accept: false);