perf: use index access to Dictionary #1621
Conversation
src/Magick.NET/Statistics/Moments.cs
Outdated
| @@ -37,10 +37,7 @@ public IChannelMoments Composite() | |||
| /// <param name="channel">The channel to get the moments for.</param> | |||
| /// <returns>The moments for the specified channel.</returns> | |||
| public IChannelMoments GetChannel(PixelChannel channel) | |||
There was a problem hiding this comment.
Should we not change the interface here instead and do IChannelMoments?
| public class TheGetChannelMethod | ||
| { | ||
| [Fact] | ||
| public void ShouldThrowExceptionWhenChannelDoesNotExist() |
| public class TheGetChannelMethod | ||
| { | ||
| [Fact] | ||
| public void ShouldThrowExceptionWhenChannelDoesNotExist() |
| using ImageMagick; | ||
| using Xunit; | ||
|
|
||
| namespace Magick.NET.Tests; | ||
|
|
||
| public partial class PerceptualHashTests | ||
| { | ||
| public class TestPerceptualHash : IPerceptualHash |
There was a problem hiding this comment.
I will move this to a separate file after merging this PR. Not yet sure where I want to put this. And maybe we could use NSubstitute instead?
There was a problem hiding this comment.
I'm used to anonymous object that can implements an Interface in PHP or Kotlin 😜
I don't know NSubstitute (this one?), but I could use it instead of this Test class.
| { | ||
| public IChannelPerceptualHash GetChannel(PixelChannel channel) | ||
| { | ||
| return null; |
| using var image = new MagickImage(Files.ImageMagickJPG); | ||
| var phash = image.PerceptualHash(); | ||
|
|
||
| Assert.Throws<NotSupportedException>(() => phash.SumSquaredDistance(new TestPerceptualHash())); |
There was a problem hiding this comment.
You can also check the message of the exception like we do in other places.
|
I can't rerun the pipeline; it seems stuck like "last time", not because of code, but for some other reasons |
|
You will probably need to rebase your PR? |
Description
👋
Theses method doesn't use the result of
TryGetValueand expect a non-null result:Magick.NET/src/Magick.NET/Statistics/Moments.cs
Lines 39 to 43 in d58472f
Magick.NET/src/Magick.NET/Statistics/PerceptualHash.cs
Lines 66 to 70 in d58472f
This one can - explicitly - return a null value:
Magick.NET/src/Magick.NET/Statistics/Statistics.cs
Lines 35 to 39 in d58472f
This PR:
TryGetValuefor the first too method and use index accessIf otherwise you prefer to not change the actual behavior (as it might be a breaking change, or just because the nullable returns is the wanted behavior), I will update the PR:
Regards.