Comments review

src/JustEat.StatsD/Buffered/StatsDUtf8Formatter.cs

@@ -4,7 +4,7 @@
 
 namespace JustEat.StatsD.Buffered
 {
-    internal class StatsDUtf8Formatter
+    internal sealed class StatsDUtf8Formatter
     {
         private readonly byte[] _utf8Prefix;
 

src/JustEat.StatsD/DisposableTimer.cs

@@ -12,16 +12,16 @@ internal sealed class DisposableTimer : IDisposableTimer
 
         public string StatName { get; set; }
 
-        public DisposableTimer(IStatsDPublisher publisher, string statName)
+        public DisposableTimer(IStatsDPublisher publisher, string bucket)
         {
             _publisher = publisher ?? throw new ArgumentNullException(nameof(publisher));
 
-            if (string.IsNullOrEmpty(statName))
+            if (string.IsNullOrEmpty(bucket))
             {
-                throw new ArgumentNullException(nameof(statName));
+                throw new ArgumentNullException(nameof(bucket));
             }
 
-            StatName = statName;
+            StatName = bucket;
             _stopwatch = Stopwatch.StartNew();
         }
 

src/JustEat.StatsD/EndpointLookups/CachedEndpointSource.cs

@@ -22,9 +22,18 @@ public class CachedEndpointSource : IEndPointSource
         /// <exception cref="ArgumentNullException">
         /// <paramref name="inner"/> is <see langword="null"/>.
         /// </exception>
+        /// <exception cref="ArgumentOutOfRangeException">
+        /// <paramref name="cacheDuration"/> is less than or equal to <see cref="TimeSpan.Zero"/>.
+        /// </exception>
         public CachedEndpointSource(IEndPointSource inner, TimeSpan cacheDuration)
         {
             _inner = inner ?? throw new ArgumentNullException(nameof(inner));
+
+            if (cacheDuration <= TimeSpan.Zero)
+            {
+                throw new ArgumentOutOfRangeException(nameof(cacheDuration), cacheDuration, "The end point cache duration must be a positive TimeSpan value.");
+            }
+
             _cachedValue = null;
             _cacheDuration = cacheDuration;
         }

src/JustEat.StatsD/EndpointLookups/DnsLookupIpEndpointSource.cs

@@ -7,7 +7,7 @@ namespace JustEat.StatsD.EndpointLookups
 {
     /// <summary>
     /// A class representing an implementation of <see cref="IEndPointSource"/> that looks up
-    /// the <see cref="EndPoint"/> for DNS hostname to resolve its IP address.
+    /// the <see cref="EndPoint"/> for a DNS hostname to resolve its IP address.
     /// </summary>
     public class DnsLookupIpEndpointSource : IEndPointSource
     {

src/JustEat.StatsD/SocketProtocol.cs

@@ -1,9 +1,11 @@
+using System.Net.Sockets;
+
 namespace JustEat.StatsD
 {
     /// <summary>
-    /// The subset of ProtocolType that are supported by SocketTransport
-    /// UDP or IP.
-    /// UDP is the default, but IP transport is required for AWS Lambdas.
+    /// An emueration defining the subset of <see cref="ProtocolType"/> values that are supported by <see cref="SocketTransport"/>.
+    /// <para />
+    /// UDP is the default, but IP transport is required for some environments such as AWS Lambda functions.
     /// </summary>
     public enum SocketProtocol
     {

src/JustEat.StatsD/TimerExtensions.cs

@@ -18,6 +18,9 @@ public static class TimerExtensions
         /// <returns>
         /// An <see cref="IDisposableTimer"/> that publishes the metric when the instance is disposed of.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static IDisposableTimer StartTimer(this IStatsDPublisher publisher, string bucket)
         {
             return new DisposableTimer(publisher, bucket);
@@ -30,6 +33,9 @@ public static IDisposableTimer StartTimer(this IStatsDPublisher publisher, strin
         /// <param name="publisher">The <see cref="IStatsDPublisher"/> to publish with.</param>
         /// <param name="bucket">The bucket to publish the timer for.</param>
         /// <param name="action">A delegate to a method whose invocation should be timed.</param>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static void Time(this IStatsDPublisher publisher, string bucket, Action action)
         {
             using (StartTimer(publisher, bucket))
@@ -45,6 +51,9 @@ public static void Time(this IStatsDPublisher publisher, string bucket, Action a
         /// <param name="publisher">The <see cref="IStatsDPublisher"/> to publish with.</param>
         /// <param name="bucket">The bucket to publish the timer for.</param>
         /// <param name="action">A delegate to a method whose invocation should be timed.</param>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static void Time(this IStatsDPublisher publisher, string bucket, Action<IDisposableTimer> action)
         {
             using (var timer = StartTimer(publisher, bucket))
@@ -63,6 +72,9 @@ public static void Time(this IStatsDPublisher publisher, string bucket, Action<I
         /// <returns>
         /// A <see cref="Task"/> representing the asynchronous operation to time.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static async Task Time(this IStatsDPublisher publisher, string bucket, Func<Task> action)
         {
             using (StartTimer(publisher, bucket))
@@ -82,6 +94,9 @@ public static async Task Time(this IStatsDPublisher publisher, string bucket, Fu
         /// <returns>
         /// A <see cref="Task"/> representing the asynchronous operation to time.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static async Task Time(this IStatsDPublisher publisher, string bucket, Func<IDisposableTimer, Task> action)
         {
             using (var timer = StartTimer(publisher, bucket))
@@ -101,6 +116,9 @@ public static async Task Time(this IStatsDPublisher publisher, string bucket, Fu
         /// <returns>
         /// The value from invoking <paramref name="func"/>.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static T Time<T>(this IStatsDPublisher publisher, string bucket, Func<T> func)
         {
             using (StartTimer(publisher, bucket))
@@ -120,6 +138,9 @@ public static T Time<T>(this IStatsDPublisher publisher, string bucket, Func<T>
         /// <returns>
         /// The value from invoking <paramref name="func"/>.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static T Time<T>(this IStatsDPublisher publisher, string bucket, Func<IDisposableTimer, T> func)
         {
             using (var timer = StartTimer(publisher, bucket))
@@ -139,6 +160,9 @@ public static T Time<T>(this IStatsDPublisher publisher, string bucket, Func<IDi
         /// <returns>
         /// A <see cref="Task"/> representing the asynchronous operation to time.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static async Task<T> Time<T>(this IStatsDPublisher publisher, string bucket, Func<Task<T>> func)
         {
             using (StartTimer(publisher, bucket))
@@ -158,6 +182,9 @@ public static async Task<T> Time<T>(this IStatsDPublisher publisher, string buck
         /// <returns>
         /// A <see cref="Task"/> representing the asynchronous operation to time.
         /// </returns>
+        /// <exception cref="ArgumentNullException">
+        /// <paramref name="publisher"/> or <paramref name="bucket"/> is <see langword="null"/>.
+        /// </exception>
         public static async Task<T> Time<T>(this IStatsDPublisher publisher, string bucket, Func<IDisposableTimer, Task<T>> func)
         {
             using (var timer = StartTimer(publisher, bucket))

tests/JustEat.StatsD.Tests/EndpointLookups/CachedEndpointSourceTests.cs

@@ -62,6 +62,21 @@ public static async Task CachedValueIsReturnedAgainAfterExpiry()
             mockInner.Verify(x => x.GetEndpoint(), Times.Exactly(2));
         }
 
+        [Theory]
+        [InlineData(0)]
+        [InlineData(-1)]
+        public static void ConstructorThrowsIfCacheDurationIsInvalid(long ticks)
+        {
+            // Arrange
+            var inner = Mock.Of<IEndPointSource>();
+            var cacheDuration = TimeSpan.FromTicks(ticks);
+
+            // Act and Assert
+            var exception = Assert.Throws<ArgumentOutOfRangeException>("cacheDuration", () => new CachedEndpointSource(inner, cacheDuration));
+
+            exception.ActualValue.ShouldBe(cacheDuration);
+        }
+
         private static IPEndPoint MakeTestIpEndPoint()
         {
             return new IPEndPoint(new IPAddress(new byte[] { 1, 2, 3, 4 }), 8125);