-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split namespaces
into Different Libraries.
#3136
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for make this in small pr
src/Neo.IO/ByteArrayComparer.cs
Outdated
: -CompareInternal(x, y); | ||
if (ReferenceEquals(x, y)) return 0; | ||
if (x is null && y is not null) return -y.Length; | ||
if (y is null && x is not null) return x.Length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If x is empty, it will return 0 that means is the same, I think that it's better to return -1 and +1 only when null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now tell if i'm wrong; i sometimes have trouble with stuff like this.
Currently flow goes
- if
x
is same instance asy
return0
- if
x
isnull
andy
is notnull
then returnx
is less thany
length. - if
y
isnull
andx
is notnull
then returnx
is greater thany
length - if
x
andy
is notnull
then return the comparison - the only thing left is: if
x
andy
isnull
then return0
aka the same.
src/Neo.IO/ByteArrayComparer.cs
Outdated
if (ReferenceEquals(x, y)) return 0; | ||
if (x is null && y is not null) return -y.Length; | ||
if (y is null && x is not null) return x.Length; | ||
if (x is not null && y is not null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is imposible to be null x and y here, we can remove this if
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if they both null
it return 0
its the last line in the method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we check if both are null twice
public int Compare(byte[]? x, byte[]? y)
{
if (ReferenceEquals(x, y)) return 0; // x == y || x == null && y == null
if (x is null && y is not null)
return _direction > 0 ? -y.Length : y.Length;
if (y is null && x is not null)
return _direction > 0 ? x.Length : -x.Length;
#pragma warning disable CS8604 // Possible null reference argument.
return _direction > 0 ?
CompareInternal(x, y) :
-CompareInternal(x, y);
#pragma warning restore CS8604 // Possible null reference argument.
}
@shargon All test pass. Updated unit tests for
|
@@ -31,7 +31,7 @@ public Logger() | |||
} | |||
} | |||
|
|||
public static event LogEventHandler Logging; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Neo.Extensions
in Neo.Vm
to avoid duplicate Neo.VM.Utility.StrictUTF8
? This can be done later.
@@ -12,6 +12,7 @@ | |||
using FluentAssertions; | |||
using Microsoft.VisualStudio.TestTools.UnitTesting; | |||
using Neo.IO; | |||
using System; | |||
|
|||
namespace Neo.UnitTests.IO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a test project to Neo.IO
and move these test to that project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I planning on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not in the same PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR moves existing code to new library. There is or should be tests already created. This PR also doesn't change anything for application besides, now having an extra dll
. In the next few PRs will be moving and checking for tests, if none than iIl create them. Just want to get the library projects started and not be to big.
@shargon
I can create one. If you would like.
src/Neo.IO/ByteArrayComparer.cs
Outdated
if (ReferenceEquals(x, y)) return 0; | ||
if (x is null && y is not null) return -y.Length; | ||
if (y is null && x is not null) return x.Length; | ||
if (x is not null && y is not null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we check if both are null twice
public int Compare(byte[]? x, byte[]? y)
{
if (ReferenceEquals(x, y)) return 0; // x == y || x == null && y == null
if (x is null && y is not null)
return _direction > 0 ? -y.Length : y.Length;
if (y is null && x is not null)
return _direction > 0 ? x.Length : -x.Length;
#pragma warning disable CS8604 // Possible null reference argument.
return _direction > 0 ?
CompareInternal(x, y) :
-CompareInternal(x, y);
#pragma warning restore CS8604 // Possible null reference argument.
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shargon thanks for fixing that up. if
statements lose me sometimes.
working on it. |
I was waiting this response #3136 (comment) |
Description
This is a small change for now. A lot more changes to come in future
PR
s. But this will force everyone to organize their code to the appropriate libraries. Allnamespace
,class
,methods
andetc
are all the same. The only difference is they may have moved to a different library. In addition, local or global variable names may have changed; these changes do not affect the functionality in any way; just following our coding practices.Please Note: This will break
neo-modules
. We will need to update after thisPR
is merged. Nevermind this won't break them unless you don't import fromneo.dll
Libraries
Neo.IO
IO
Neo.Extensions
neo
. Including but not limited to other libraries. Notest
functionality allowed. Please Note: you can include this library in tests; just don't add code that extendstests
.Type of change
How Has This Been Tested?
Checklist: