Skip to content

Commit

Permalink
Slight performance optimization by using ExceptionHelper to throw Arg…
Browse files Browse the repository at this point in the history
…umentNullExceptions #21
  • Loading branch information
Havunen committed Jul 20, 2023
1 parent 043759d commit 35a8df9
Show file tree
Hide file tree
Showing 16 changed files with 197 additions and 468 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Dynamic;
using System.Text.Json;
using SystemTextJsonPatch.Exceptions;
using Xunit;

namespace SystemTextJsonPatch.IntegrationTests;
Expand Down Expand Up @@ -338,7 +339,11 @@ public string StringProperty
{
get => _stringProperty;

set => _stringProperty = value ?? throw new ArgumentNullException(nameof(value));
set
{
ExceptionHelper.ThrowIfNull(value, nameof(value));
_stringProperty = value;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion SystemTextJsonPatch.Tests/SystemTextJsonPatch.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<IsPackable>false</IsPackable>
<IsPublishable>false</IsPublishable>
<IsUnitTestProject>True</IsUnitTestProject>
<LangVersion>10</LangVersion>
<LangVersion>11</LangVersion>
<RepositoryUrl>https://github.com/Havunen/SystemTextJsonPatch.git</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<AnalysisLevel>6.0-all</AnalysisLevel>
Expand Down
9 changes: 3 additions & 6 deletions SystemTextJsonPatch/Adapters/AdapterFactory.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System;
using System.Collections;
using System.Collections;
using System.Text.Json;
using System.Text.Json.Nodes;
using SystemTextJsonPatch.Exceptions;
using SystemTextJsonPatch.Internal;

namespace SystemTextJsonPatch.Adapters;
Expand All @@ -18,10 +18,7 @@ public class AdapterFactory : IAdapterFactory
public IAdapter Create(object target, JsonSerializerOptions options)
#pragma warning restore PUB0001
{
if (target == null)
{
throw new ArgumentNullException(nameof(target));
}
ExceptionHelper.ThrowIfNull(target, nameof(target));

if (target is JsonObject)
{
Expand Down
121 changes: 31 additions & 90 deletions SystemTextJsonPatch/Adapters/ObjectAdapter.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Text.Json;
using SystemTextJsonPatch.Exceptions;
using SystemTextJsonPatch.Internal;
using SystemTextJsonPatch.Operations;

Expand Down Expand Up @@ -31,9 +32,12 @@ public class ObjectAdapter : IObjectAdapterWithTest
Action<JsonPatchError>? logErrorAction,
IAdapterFactory adapterFactory)
{
Options = options ?? throw new ArgumentNullException(nameof(options));
ExceptionHelper.ThrowIfNull(options, nameof(options));
ExceptionHelper.ThrowIfNull(adapterFactory, nameof(adapterFactory));

Options = options;
LogErrorAction = logErrorAction;
AdapterFactory = adapterFactory ?? throw new ArgumentNullException(nameof(adapterFactory));
AdapterFactory = adapterFactory;
}

public JsonSerializerOptions Options { get; }
Expand All @@ -50,17 +54,10 @@ public class ObjectAdapter : IObjectAdapterWithTest

public void Add(Operation? operation, object objectToApplyTo)
{
if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}
ExceptionHelper.ThrowIfNull(operation, nameof(operation));
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));

Add(operation.Path, operation.Value, objectToApplyTo, operation);
Add(operation.Path, operation.Value, objectToApplyTo, operation);

Check warning on line 60 in SystemTextJsonPatch/Adapters/ObjectAdapter.cs

View workflow job for this annotation

GitHub Actions / build and test

Dereference of a possibly null reference.

Check warning on line 60 in SystemTextJsonPatch/Adapters/ObjectAdapter.cs

View workflow job for this annotation

GitHub Actions / build and test

Possible null reference argument for parameter 'path' in 'void ObjectAdapter.Add(string path, object? value, object objectToApplyTo, Operation operation)'.
}

/// <summary>
Expand All @@ -74,20 +71,10 @@ public void Add(Operation? operation, object objectToApplyTo)
Operation operation
)
{
if (path == null)
{
throw new ArgumentNullException(nameof(path));
}

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}
ExceptionHelper.ThrowIfNull(path, nameof(path));
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));
ExceptionHelper.ThrowIfNull(operation, nameof(operation));

if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}

var parsedPath = new ParsedPath(path);
var visitor = new ObjectVisitor(parsedPath, Options, AdapterFactory);
Expand All @@ -110,17 +97,10 @@ Operation operation

public void Move(Operation operation, object objectToApplyTo)
{
if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));
ExceptionHelper.ThrowIfNull(operation, nameof(operation));

// Get value at 'from' location and add that value to the 'path' location
// Get value at 'from' location and add that value to the 'path' location
if (TryGetValue(operation.From, objectToApplyTo, operation, out var propertyValue))
{
// remove that value
Expand All @@ -136,17 +116,10 @@ public void Move(Operation operation, object objectToApplyTo)

public void Remove(Operation operation, object objectToApplyTo)
{
if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}
ExceptionHelper.ThrowIfNull(operation, nameof(operation));
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));

Remove(operation.Path, objectToApplyTo, operation);
Remove(operation.Path, objectToApplyTo, operation);
}

/// <summary>
Expand Down Expand Up @@ -179,17 +152,10 @@ private void Remove(string? path, object objectToApplyTo, Operation operationToR

public void Replace(Operation operation, object objectToApplyTo)
{
if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}
ExceptionHelper.ThrowIfNull(operation, nameof(operation));
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));

var parsedPath = new ParsedPath(operation.Path);
var parsedPath = new ParsedPath(operation.Path);
var visitor = new ObjectVisitor(parsedPath, Options, AdapterFactory);

var target = objectToApplyTo;
Expand All @@ -210,18 +176,11 @@ public void Replace(Operation operation, object objectToApplyTo)

public void Copy(Operation operation, object objectToApplyTo)
{
if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}
ExceptionHelper.ThrowIfNull(operation, nameof(operation));
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}

// Get value at 'from' location and add that value to the 'path' location
if (TryGetValue(operation.From, objectToApplyTo, operation, out var propertyValue))
// Get value at 'from' location and add that value to the 'path' location
if (TryGetValue(operation.From, objectToApplyTo, operation, out var propertyValue))
{
// Create deep copy
if (ConversionResultProvider.TryCopyTo(propertyValue, propertyValue?.GetType(), this.Options, out object? convertedValue))
Expand All @@ -242,17 +201,10 @@ public void Copy(Operation operation, object objectToApplyTo)

public void Test(Operation operation, object objectToApplyTo)
{
if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}
ExceptionHelper.ThrowIfNull(operation, nameof(operation));
ExceptionHelper.ThrowIfNull(objectToApplyTo, nameof(objectToApplyTo));

if (objectToApplyTo == null)
{
throw new ArgumentNullException(nameof(objectToApplyTo));
}

var parsedPath = new ParsedPath(operation.Path);
var parsedPath = new ParsedPath(operation.Path);
var visitor = new ObjectVisitor(parsedPath, Options, AdapterFactory);

var target = objectToApplyTo;
Expand All @@ -277,20 +229,9 @@ public void Test(Operation operation, object objectToApplyTo)
Operation operation,
out object? propertyValue)
{
if (fromLocation == null)
{
throw new ArgumentNullException(nameof(fromLocation));
}

if (objectToGetValueFrom == null)
{
throw new ArgumentNullException(nameof(objectToGetValueFrom));
}

if (operation == null)
{
throw new ArgumentNullException(nameof(operation));
}
ExceptionHelper.ThrowIfNull(fromLocation, nameof(fromLocation));
ExceptionHelper.ThrowIfNull(operation, nameof(operation));
ExceptionHelper.ThrowIfNull(objectToGetValueFrom, nameof(objectToGetValueFrom));

propertyValue = null;

Expand Down
19 changes: 19 additions & 0 deletions SystemTextJsonPatch/Exceptions/ExceptionHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;

namespace SystemTextJsonPatch.Exceptions
{
internal class ExceptionHelper
{
public static void ThrowIfNull(object? argument, string paramName)
{
#if NETSTANDARD2_0
if (argument == null)
{
throw new ArgumentNullException(nameof(argument));
}
#else
ArgumentNullException.ThrowIfNull(argument, paramName);
#endif
}
}
}
29 changes: 0 additions & 29 deletions SystemTextJsonPatch/Helpers/JsonPatchProperty.cs

This file was deleted.

14 changes: 4 additions & 10 deletions SystemTextJsonPatch/Internal/ListAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Text.Json;
using SystemTextJsonPatch.Exceptions;

namespace SystemTextJsonPatch.Internal;

Expand Down Expand Up @@ -270,17 +271,10 @@ private static bool TryGetListTypeArgument(IList list, out Type? listTypeArgumen

private static Type? ExtractGenericInterface(Type queryType, Type interfaceType)
{
if (queryType == null)
{
throw new ArgumentNullException(nameof(queryType));
}

if (interfaceType == null)
{
throw new ArgumentNullException(nameof(interfaceType));
}
ExceptionHelper.ThrowIfNull(queryType, nameof(queryType));
ExceptionHelper.ThrowIfNull(interfaceType, nameof(interfaceType));

if (IsGenericInstantiation(queryType, interfaceType))
if (IsGenericInstantiation(queryType, interfaceType))
{
// queryType matches (i.e. is a closed generic type created from) the open generic type.
return queryType;
Expand Down
13 changes: 8 additions & 5 deletions SystemTextJsonPatch/Internal/ObjectVisitor.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using System;
using System.Text.Json;
using System.Text.Json;
using SystemTextJsonPatch.Adapters;
using SystemTextJsonPatch.Exceptions;

namespace SystemTextJsonPatch.Internal;

Expand All @@ -21,9 +21,12 @@ public ObjectVisitor(ParsedPath path, JsonSerializerOptions options)

public ObjectVisitor(ParsedPath path, JsonSerializerOptions options, IAdapterFactory adapterFactory)
{
_path = path;
_options = options ?? throw new ArgumentNullException(nameof(options));
_adapterFactory = adapterFactory ?? throw new ArgumentNullException(nameof(adapterFactory));
ExceptionHelper.ThrowIfNull(options, nameof(options));
ExceptionHelper.ThrowIfNull(adapterFactory, nameof(adapterFactory));

_path = path;
_options = options;
_adapterFactory = adapterFactory;
}

public bool TryVisit(ref object target, out IAdapter? adapter, out string? errorMessage)
Expand Down
7 changes: 2 additions & 5 deletions SystemTextJsonPatch/Internal/ParsedPath.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,9 @@ namespace SystemTextJsonPatch.Internal;

public ParsedPath(string? path)
{
if (path == null)
{
throw new ArgumentNullException(nameof(path));
}
ExceptionHelper.ThrowIfNull(path, nameof(path));

_segments = ParsePath(path);
_segments = ParsePath(path!);
}

public string? LastSegment
Expand Down
Loading

0 comments on commit 35a8df9

Please sign in to comment.