Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Incorporate code review feedback for ResultConverters

- Reuse singleton converter instances where possible.
- Throw on null controllerContext
  • Loading branch information...
commit 6bda9407809a28e6ebef4bc97fd88ca98b68e544 1 parent f95cdef
Marcin Dobosz authored April 05, 2012
11  src/System.Web.Http/Controllers/HttpActionDescriptor.cs
@@ -27,6 +27,9 @@ public abstract class HttpActionDescriptor
27 27
 
28 28
         private HttpActionBinding _actionBinding;
29 29
 
  30
+        private static readonly ResponseMessageResultConverter _responseMessageResultConverter = new ResponseMessageResultConverter();
  31
+        private static readonly VoidResultConverter _voidResultConverter = new VoidResultConverter();
  32
+
30 33
         protected HttpActionDescriptor()
31 34
         {
32 35
             _filterPipeline = new Lazy<Collection<FilterInfo>>(InitializeFilterPipeline);
@@ -62,7 +65,7 @@ public HttpConfiguration Configuration
62 65
         public virtual HttpActionBinding ActionBinding
63 66
         {
64 67
             get
65  
-            {                
  68
+            {
66 69
                 if (_actionBinding == null)
67 70
                 {
68 71
                     IActionValueBinder actionValueBinder = _controllerDescriptor.ActionValueBinder;
@@ -73,7 +76,7 @@ public virtual HttpActionBinding ActionBinding
73 76
             }
74 77
             set
75 78
             {
76  
-                if (value == null) 
  79
+                if (value == null)
77 80
                 {
78 81
                     throw Error.PropertyNull();
79 82
                 }
@@ -165,11 +168,11 @@ internal static IActionResultConverter GetResultConverter(Type type)
165 168
 
166 169
             if (type == null)
167 170
             {
168  
-                return new VoidResultConverter();
  171
+                return _voidResultConverter;
169 172
             }
170 173
             else if (typeof(HttpResponseMessage).IsAssignableFrom(type))
171 174
             {
172  
-                return new ResponseMessageResultConverter();
  175
+                return _responseMessageResultConverter;
173 176
             }
174 177
             else
175 178
             {
5  src/System.Web.Http/Controllers/ResponseMessageResultConverter.cs
@@ -12,6 +12,11 @@ public class ResponseMessageResultConverter : IActionResultConverter
12 12
     {
13 13
         public HttpResponseMessage Convert(HttpControllerContext controllerContext, object actionResult)
14 14
         {
  15
+            if (controllerContext == null)
  16
+            {
  17
+                throw Error.ArgumentNull("controllerContext");
  18
+            }
  19
+
15 20
             HttpResponseMessage response = (HttpResponseMessage)actionResult;
16 21
             if (response == null)
17 22
             {
5  src/System.Web.Http/Controllers/ValueResultConverter.cs
@@ -13,6 +13,11 @@ public class ValueResultConverter<T> : IActionResultConverter
13 13
     {
14 14
         public HttpResponseMessage Convert(HttpControllerContext controllerContext, object actionResult)
15 15
         {
  16
+            if (controllerContext == null)
  17
+            {
  18
+                throw Error.ArgumentNull("controllerContext");
  19
+            }
  20
+
16 21
             HttpResponseMessage resultAsResponse = actionResult as HttpResponseMessage;
17 22
             if (resultAsResponse != null)
18 23
             {
5  src/System.Web.Http/Controllers/VoidResultConverter.cs
@@ -12,6 +12,11 @@ public class VoidResultConverter : IActionResultConverter
12 12
     {
13 13
         public HttpResponseMessage Convert(HttpControllerContext controllerContext, object actionResult)
14 14
         {
  15
+            if (controllerContext == null)
  16
+            {
  17
+                throw Error.ArgumentNull("controllerContext");
  18
+            }
  19
+
15 20
             Contract.Assert(actionResult == null);
16 21
             return controllerContext.Request.CreateResponse();
17 22
         }
6  test/System.Web.Http.Test/Controllers/ResponseMessageResultConverterTest.cs
@@ -30,6 +30,12 @@ public void Convert_WhenValueIsResponseMessage_ReturnsResponseMessageWithRequest
30 30
         }
31 31
 
32 32
         [Fact]
  33
+        public void Convert_WhenContextIsNull_Throws()
  34
+        {
  35
+            Assert.ThrowsArgumentNull(() => _converter.Convert(controllerContext: null, actionResult: new HttpResponseMessage()), "controllerContext");
  36
+        }
  37
+
  38
+        [Fact]
33 39
         public void Convert_WhenValueIsNull_Throws()
34 40
         {
35 41
             Assert.Throws<InvalidOperationException>(() => _converter.Convert(_context, null),
6  test/System.Web.Http.Test/Controllers/ValueResultConverterTest.cs
@@ -22,6 +22,12 @@ public ValueResultConverterTest()
22 22
         }
23 23
 
24 24
         [Fact]
  25
+        public void Convert_WhenContextIsNull_Throws()
  26
+        {
  27
+            Assert.ThrowsArgumentNull(() => _objectValueConverter.Convert(controllerContext: null, actionResult: new object()), "controllerContext");
  28
+        }
  29
+
  30
+        [Fact]
25 31
         public void Convert_WhenValueTypeIsNotCompatible_Throws()
26 32
         {
27 33
             Assert.Throws<InvalidCastException>(() => _animalValueConverter.Convert(_context, new object()),
6  test/System.Web.Http.Test/Controllers/VoidResultConverterTest.cs
@@ -20,6 +20,12 @@ public VoidResultConverterTest()
20 20
         }
21 21
 
22 22
         [Fact]
  23
+        public void Convert_WhenContextIsNull_Throws()
  24
+        {
  25
+            Assert.ThrowsArgumentNull(() => _converter.Convert(controllerContext: null, actionResult: null), "controllerContext");
  26
+        }
  27
+
  28
+        [Fact]
23 29
         public void Convert_ReturnsResponseMessageWithRequestAssigned()
24 30
         {
25 31
             var result = _converter.Convert(_context, null);

0 notes on commit 6bda940

Please sign in to comment.
Something went wrong with that request. Please try again.