Improve Code Quality 3 (#805)

* Throw when trying to inverse a matrix with a determinant of 0 * Optimize Hex.GetString on .NET * Updates tests for Matrix3x3.Inverse() change * Eliminate allocation in InternalStringExtensions * Use vectorized Span.Fill method * Eliminate various string allocations when parsing numbers * Remove unused using statements * Fix Matrix3x3 Equals nullability
UglyToad · Mar 17, 2024 · 69e2b7b · 69e2b7b
1 parent a412a23
commit 69e2b7b
Show file tree

Hide file tree

Showing 12 changed files with 55 additions and 64 deletions.
diff --git a/src/UglyToad.PdfPig.Tests/Util/InternalStringExtensionsTests.cs b/src/UglyToad.PdfPig.Tests/Util/InternalStringExtensionsTests.cs
@@ -15,17 +15,17 @@ public class InternalStringExtensionsTests
         [InlineData("pineapple", "apple", 4, true)]
         public void StartsWithOffset(string input, string start, int offset, bool expected)
         {
-            var result = input.StartsWithOffset(start, offset);
+            var result = input.StartsWithOffset(start.AsSpan(), offset);
 
             Assert.Equal(expected, result);
         }
 
         [Fact]
         public void StartsWithOffset_NegativeOffset_Throws()
         {
-            Action action = () => "any".StartsWithOffset("x", -1);
+            Action action = () => "any".StartsWithOffset("x".AsSpan(), -1);
 
             Assert.Throws<ArgumentOutOfRangeException>(action);
         }
     }
-}
+}
diff --git a/src/UglyToad.PdfPig.Tests/Util/Matrix3x3Tests.cs b/src/UglyToad.PdfPig.Tests/Util/Matrix3x3Tests.cs
@@ -106,15 +106,14 @@ public void InverseReturnsMatrixIfPossible()
         }
 
         [Fact]
-        public void InverseReturnsNullIfNotPossible()
+        public void InverseThrowsNotPossible()
         {
             var matrix = new Matrix3x3(
                 1, 2, 3,
                 4, 5, 6,
                 7, 8, 9);
 
-            var inverse = matrix.Inverse();
-            Assert.Null(inverse);
+            Assert.Throws<InvalidOperationException>(() => matrix.Inverse());
         }
     }
 }
diff --git a/src/UglyToad.PdfPig/Filters/CcittFaxDecoderStream.cs b/src/UglyToad.PdfPig/Filters/CcittFaxDecoderStream.cs
@@ -3,7 +3,6 @@
     using System;
     using System.IO;
     using IO;
-    using Util;
 
     /// <summary>
     /// CCITT Modified Huffman RLE, Group 3 (T4) and Group 4 (T6) fax compression.
@@ -426,7 +425,7 @@ public override int Read(byte[] b, int off, int len)
         {
             if (decodedLength < 0)
             {
-                ArrayHelper.Fill(b, off, off + len, (byte)0x0);
+                b.AsSpan(off, len).Fill(0x0);
                 return len;
             }
 
@@ -436,7 +435,7 @@ public override int Read(byte[] b, int off, int len)
 
                 if (decodedLength < 0)
                 {
-                    ArrayHelper.Fill(b, off, off + len, (byte)0x0);
+                    b.AsSpan(off, len).Fill(0x0);
                     return len;
                 }
             }

diff --git a/src/UglyToad.PdfPig/Graphics/Colors/ChromaticAdaptation.cs b/src/UglyToad.PdfPig/Graphics/Colors/ChromaticAdaptation.cs
@@ -18,7 +18,7 @@ public ChromaticAdaptation(
             Method method = Method.Bradford)
         {
             var coneReponseDomain = GetConeResponseDomain(method)!;
-            var inverseConeResponseDomain = coneReponseDomain.Inverse()!;
+            var inverseConeResponseDomain = coneReponseDomain.Inverse();
             var (ρS, γS, βS) = coneReponseDomain.Multiply(sourceReferenceWhite);
 
             var (ρD, γD, βD) = coneReponseDomain.Multiply(destinationReferenceWhite);

diff --git a/src/UglyToad.PdfPig/Parser/FileStructure/FileHeaderParser.cs b/src/UglyToad.PdfPig/Parser/FileStructure/FileHeaderParser.cs
@@ -8,6 +8,7 @@
     using Logging;
     using Tokenization.Scanner;
     using Tokens;
+    using UglyToad.PdfPig.Util;
 
     /// <summary>
     /// Used to retrieve the version header from the PDF file.
@@ -70,7 +71,7 @@ private static HeaderVersion GetHeaderVersionAndResetScanner(CommentToken commen
 
             const int toDoubleStartLength = 4;
 
-            if (!double.TryParse(comment.Data.Substring(toDoubleStartLength),
+            if (!double.TryParse(comment.Data.AsSpanOrSubstring(toDoubleStartLength),
                 NumberStyles.Number,
                 CultureInfo.InvariantCulture,
                 out var version))
@@ -118,7 +119,7 @@ private static bool TryBruteForceVersionLocation(long startPosition, IInputBytes
 
                 if (actualIndex >= 0 && content.Length - actualIndex >= versionLength)
                 {
-                    var numberPart = content.Substring(actualIndex + 5, 3);
+                    var numberPart = content.AsSpanOrSubstring(actualIndex + 5, 3);
                     if (double.TryParse(
                             numberPart,
                             NumberStyles.Number,

diff --git a/src/UglyToad.PdfPig/Util/ArrayHelper.cs b/src/UglyToad.PdfPig/Util/ArrayHelper.cs
diff --git a/src/UglyToad.PdfPig/Util/DateFormatHelper.cs b/src/UglyToad.PdfPig/Util/DateFormatHelper.cs
@@ -1,6 +1,7 @@
 namespace UglyToad.PdfPig.Util
 {
     using System;
+    using System.Globalization;
 
     /// <summary>
     /// Helper class for dates.
@@ -53,7 +54,7 @@ bool IsWithinRange(int val, int min, int max)
                     return false;
                 }
 
-                if (!int.TryParse(s.Substring(location, 4), out var year))
+                if (!int.TryParse(s.AsSpanOrSubstring(location, 4), NumberStyles.Integer, CultureInfo.InvariantCulture, out var year))
                 {
                     return false;
                 }
@@ -72,7 +73,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!int.TryParse(s.Substring(location, 2), out var month)
+                if (!int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var month)
                     || !IsWithinRange(month, 1, 12))
                 {
                     return false;
@@ -92,7 +93,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!int.TryParse(s.Substring(location, 2), out var day)
+                if (!int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var day)
                     || !IsWithinRange(day, 1, 31))
                 {
                     return false;
@@ -112,7 +113,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!int.TryParse(s.Substring(location, 2), out var hour)
+                if (!int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var hour)
                     || !IsWithinRange(hour, 0, 23))
                 {
                     return false;
@@ -132,7 +133,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!int.TryParse(s.Substring(location, 2), out var minute)
+                if (!int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var minute)
                     || !IsWithinRange(minute, 0, 59))
                 {
                     return false;
@@ -152,7 +153,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!int.TryParse(s.Substring(location, 2), out var second)
+                if (!int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var second)
                     || !IsWithinRange(second, 0, 59))
                 {
                     return false;
@@ -189,7 +190,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!HasRemainingCharacters(location, 3) || !int.TryParse(s.Substring(location, 2), out var hoursOffset)
+                if (!HasRemainingCharacters(location, 3) || !int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var hoursOffset)
                                                          || s[location + 2] != '\''
                                                          || !IsWithinRange(hoursOffset, 0, 23))
                 {
@@ -205,7 +206,7 @@ bool IsWithinRange(int val, int min, int max)
                     return true;
                 }
 
-                if (!HasRemainingCharacters(location, 3) || !int.TryParse(s.Substring(location, 2), out var minutesOffset)
+                if (!HasRemainingCharacters(location, 3) || !int.TryParse(s.AsSpanOrSubstring(location, 2), NumberStyles.Integer, CultureInfo.InvariantCulture, out var minutesOffset)
                                                          || s[location + 2] != '\''
                                                          || !IsWithinRange(minutesOffset, 0, 59))
                 {

diff --git a/src/UglyToad.PdfPig/Util/Hex.cs b/src/UglyToad.PdfPig/Util/Hex.cs
@@ -23,6 +23,9 @@ internal static class Hex
         /// </summary>
         public static string GetString(ReadOnlySpan<byte> bytes)
         {
+#if NET6_0_OR_GREATER
+            return Convert.ToHexString(bytes); 
+#else
             var stringBuilder = new StringBuilder(bytes.Length * 2);
 
             foreach (var b in bytes)
@@ -31,6 +34,7 @@ public static string GetString(ReadOnlySpan<byte> bytes)
             }
 
             return stringBuilder.ToString();
+#endif
         }
 
         private static int GetHighNibble(byte b)

diff --git a/src/UglyToad.PdfPig/Util/InternalStringExtensions.cs b/src/UglyToad.PdfPig/Util/InternalStringExtensions.cs
@@ -4,7 +4,7 @@
 
     internal static class InternalStringExtensions
     {
-        public static bool StartsWithOffset(this string value, string start, int offset)
+        public static bool StartsWithOffset(this string value, ReadOnlySpan<char> start, int offset)
         {
             if (offset < 0)
             {
@@ -13,7 +13,7 @@ public static bool StartsWithOffset(this string value, string start, int offset)
 
             if (value is null)
             {
-                if (start is null && offset == 0)
+                if (start.Length is 0 && offset == 0)
                 {
                     return true;
                 }
@@ -26,7 +26,29 @@ public static bool StartsWithOffset(this string value, string start, int offset)
                 return false;
             }
 
-            return value.Substring(offset).StartsWith(start);
+            return value.AsSpan(offset).StartsWith(start, StringComparison.Ordinal);
         }
+
+#if NET
+    public static ReadOnlySpan<char> AsSpanOrSubstring(this string text, int start)
+    {
+        return text.AsSpan(start);
+    }
+
+    public static ReadOnlySpan<char> AsSpanOrSubstring(this string text, int start, int length)
+    {
+        return text.AsSpan(start, length);
+    }
+#else
+        public static string AsSpanOrSubstring(this string text, int start)
+        {
+            return text.Substring(start);
+        }
+
+        public static string AsSpanOrSubstring(this string text, int start, int length)
+        {
+            return text.Substring(start, length);
+        }
+#endif
     }
 }
diff --git a/src/UglyToad.PdfPig/Util/Matrix3x3.cs b/src/UglyToad.PdfPig/Util/Matrix3x3.cs
@@ -4,7 +4,7 @@
 
 namespace UglyToad.PdfPig.Util
 {
-    internal class Matrix3x3 : IEnumerable<double>, IEquatable<Matrix3x3>
+    internal sealed class Matrix3x3 : IEnumerable<double>, IEquatable<Matrix3x3>
     {
         /// <summary>
         /// The identity matrix. The result of multiplying a matrix with
@@ -69,14 +69,14 @@ public IEnumerator<double> GetEnumerator()
         ///
         /// If an inverse matrix does not exist, null is returned.
         /// </summary>
-        public Matrix3x3? Inverse()
+        public Matrix3x3 Inverse()
         {
             var determinant = GetDeterminant();
 
             // No inverse matrix exists when determinant is zero
             if (determinant == 0)
             {
-                return null;
+                throw new InvalidOperationException("May not inverse a matrix with a determinant of 0.");
             }
 
             var transposed = Transpose();
@@ -162,7 +162,7 @@ public override bool Equals(object? obj)
         public bool Equals(Matrix3x3? other)
         {
             if (other is null)
-            { 
+            {
                 return false;
             }
 

diff --git a/src/UglyToad.PdfPig/Writer/PdfA1ARuleBuilder.cs b/src/UglyToad.PdfPig/Writer/PdfA1ARuleBuilder.cs
@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using UglyToad.PdfPig.Tokens;
 
 namespace UglyToad.PdfPig.Writer

diff --git a/src/UglyToad.PdfPig/Writer/PdfWriterType.cs b/src/UglyToad.PdfPig/Writer/PdfWriterType.cs
@@ -1,9 +1,5 @@
 namespace UglyToad.PdfPig.Writer
 {
-    using System;
-    using System.Collections.Generic;
-    using System.Text;
-
     /// <summary>
     /// Type of pdf writer to use.
     /// </summary>
@@ -18,4 +14,4 @@ public enum PdfWriterType
         /// </summary>
         ObjectInMemoryDedup
     }
-}
+}