Skip to content

nullable ParamConverters#135

Merged
twogood merged 1 commit intomainfrom
nullable-paramconverter-2
Oct 28, 2025
Merged

nullable ParamConverters#135
twogood merged 1 commit intomainfrom
nullable-paramconverter-2

Conversation

@twogood
Copy link
Owner

@twogood twogood commented Oct 28, 2025

Summary by CodeRabbit

  • Refactor
    • Enhanced null-safety handling in parameter converters. The parameter conversion layer now properly accepts and handles null values, improving robustness when processing optional or empty parameters.

@twogood twogood self-assigned this Oct 28, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

The pull request enables nullable reference types across ParamConverter implementations by removing #nullable disable directives, updating the IParamConverter interface and all implementations to accept nullable object parameters in ToString methods, and improving null-handling logic using null-coalescing operators.

Changes

Cohort / File(s) Summary
ParamConverter Interface
Activout.RestClient/ParamConverter/IParamConverter.cs
Updated ToString method signature to accept nullable object parameter (object?) to propagate nullability requirements through implementations.
ParamConverter Implementations
Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs, Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs, Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs
Removed #nullable disable directives and updated ToString signatures to accept nullable objects (object?); added null-handling logic returning empty strings for null inputs. ToStringParamConverter also adopted file-scoped namespace syntax with semicolon.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Note: Changes follow a consistent refactoring pattern (enable nullable reference types, update method signatures, improve null-handling). Verify that all implementations handle null values appropriately and that downstream code expecting non-null ToString results is unaffected.

Poem

🐰 Nullable types now shine so bright,
Null-coalescing fixes the plight,
Interfaces and implementations aligned,
Null safety clearly defined! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "nullable ParamConverters" is directly related to the main changes in the changeset. The PR updates the IParamConverter interface and all its implementations (DateTimeEpochParamConverter, DateTimeIso8601ParamConverter, and ToStringParamConverter) to support nullable reference types by changing method signatures from object value to object? value and removing #nullable disable directives. The title accurately and concisely conveys the primary objective of these changes, and a teammate scanning the repository history would immediately understand that this PR introduces nullable reference type support to the ParamConverter implementations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nullable-paramconverter-2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs (1)

22-25: Apply coding guidelines: file-scoped namespace and expression-bodied member.

Per the coding guidelines, this file should use a file-scoped namespace and the simple ToString method could use an expression-bodied member for conciseness.

Apply this diff to align with coding guidelines:

-namespace Activout.RestClient.ParamConverter.Implementation
-{
+namespace Activout.RestClient.ParamConverter.Implementation;
+
     internal static class Extensions
     {
         // https://stackoverflow.com/questions/2576/how-do-you-convert-epoch-time-in-c
         public static long ToUnixTime(this DateTime date)
         {
             return (date.ToUniversalTime().Ticks - 621355968000000000) / 10000000;
         }
     }
 
     public class DateTimeEpochParamConverter : IParamConverter
     {
         public bool CanConvert(Type type, ParameterInfo parameterInfo)
         {
             return type == typeof(DateTime);
         }
 
-        public string ToString(object? value)
-        {
-            return value == null ? "" : ((DateTime)value).ToUnixTime().ToString();
-        }
+        public string ToString(object? value) =>
+            value == null ? "" : ((DateTime)value).ToUnixTime().ToString();
     }
-}

As per coding guidelines.

Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs (1)

13-16: Apply coding guidelines: file-scoped namespace and expression-bodied member.

Similar to DateTimeEpochParamConverter, this file should use a file-scoped namespace and the ToString method could use an expression-bodied member.

Apply this diff to align with coding guidelines:

-namespace Activout.RestClient.ParamConverter.Implementation
-{
+namespace Activout.RestClient.ParamConverter.Implementation;
+
     public class DateTimeIso8601ParamConverter : IParamConverter
     {
         public bool CanConvert(Type type, ParameterInfo parameterInfo)
         {
             return type == typeof(DateTime);
         }
 
-        public string ToString(object? value)
-        {
-            return value == null ? "" : ((DateTime)value).ToString("o");
-        }
+        public string ToString(object? value) =>
+            value == null ? "" : ((DateTime)value).ToString("o");
     }
-}

As per coding guidelines.

Activout.RestClient/ParamConverter/IParamConverter.cs (1)

9-9: Interface signature update enables nullable reference types correctly.

The change from object value to object? value appropriately enables nullable reference types across all ParamConverter implementations.

Consider using a file-scoped namespace for consistency with coding guidelines and ToStringParamConverter.cs:

-namespace Activout.RestClient.ParamConverter
-{
+namespace Activout.RestClient.ParamConverter;
+
     public interface IParamConverter
     {
         bool CanConvert(Type type, ParameterInfo parameterInfo);
         string ToString(object? value);
     }
-}

As per coding guidelines.

Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (1)

8-16: Consider expression-bodied members for consistency.

Both methods are simple one-liners that could use expression-bodied syntax per coding guidelines. Additionally, note that this file uses the null-coalescing operator (??) while DateTimeEpochParamConverter and DateTimeIso8601ParamConverter use ternary operators for null checks—consider standardizing the approach across all converters.

Apply this diff to use expression-bodied members:

-    public bool CanConvert(Type type, ParameterInfo parameterInfo)
-    {
-        return true;
-    }
+    public bool CanConvert(Type type, ParameterInfo parameterInfo) => true;
 
-    public string ToString(object? value)
-    {
-        return value?.ToString() ?? "";
-    }
+    public string ToString(object? value) => value?.ToString() ?? "";

As per coding guidelines.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8654030 and 036bad1.

📒 Files selected for processing (4)
  • Activout.RestClient/ParamConverter/IParamConverter.cs (1 hunks)
  • Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs (1 hunks)
  • Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs (1 hunks)
  • Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.cs: Use [] list syntax for collections
Use file-scoped namespaces
Use primary constructors wherever possible
Use records for data transfer objects (DTOs) and immutable data structures
Use var for local variable declarations when possible
Use expression-bodied members for simple methods and properties
Use pattern matching for type checks and deconstruction
Use using statements for resource management
Use async and await for asynchronous programming
Never include "Async" in method names
Only include comments that explain why something is done, not what is done

Files:

  • Activout.RestClient/ParamConverter/IParamConverter.cs
  • Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs
  • Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs
  • Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs
🧬 Code graph analysis (4)
Activout.RestClient/ParamConverter/IParamConverter.cs (4)
Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs (1)
  • ToString (22-25)
Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs (1)
  • ToString (13-16)
Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (1)
  • ToString (13-16)
Activout.RestClient.Test/ParamConverterTests.cs (1)
  • ToString (132-135)
Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs (3)
Activout.RestClient/ParamConverter/IParamConverter.cs (1)
  • ToString (9-9)
Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs (1)
  • ToString (22-25)
Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (1)
  • ToString (13-16)
Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs (4)
Activout.RestClient/ParamConverter/IParamConverter.cs (1)
  • ToString (9-9)
Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs (1)
  • ToString (13-16)
Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (1)
  • ToString (13-16)
Activout.RestClient.Test/ParamConverterTests.cs (1)
  • ToString (132-135)
Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (5)
Activout.RestClient/Implementation/RequestHandler.cs (2)
  • IParamConverter (120-131)
  • Type (160-167)
Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs (1)
  • IParamConverter (19-30)
Activout.RestClient/ParamConverter/IParamConverter.cs (2)
  • CanConvert (8-8)
  • ToString (9-9)
Activout.RestClient/ParamConverter/Implementation/DateTimeEpochParamConverter.cs (2)
  • CanConvert (17-20)
  • ToString (22-25)
Activout.RestClient.Test/ParamConverterTests.cs (2)
  • CanConvert (122-130)
  • ToString (132-135)

@twogood twogood merged commit 932547e into main Oct 28, 2025
9 checks passed
@twogood twogood deleted the nullable-paramconverter-2 branch October 28, 2025 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments