-
Notifications
You must be signed in to change notification settings - Fork 143
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
Add BigInteger to ToBencodexValue() #159
Add BigInteger to ToBencodexValue() #159
Conversation
@@ -115,6 +122,7 @@ protected Person(SerializationInfo info, StreamingContext context) | |||
.ToList(); | |||
Secret = info.GetString("Secret"); | |||
IsStudent = info.GetBoolean("IsStudent"); | |||
Assets = (BigInteger)info.GetValue("Assets", typeof(BigInteger)); |
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 already have GetValue<T>()
, so you can use it.
Codecov Report
@@ Coverage Diff @@
## master #159 +/- ##
==========================================
- Coverage 87.06% 84.16% -2.91%
==========================================
Files 69 69
Lines 3163 3164 +1
==========================================
- Hits 2754 2663 -91
- Misses 409 501 +92
|
@@ -109,12 +116,12 @@ protected Person(SerializationInfo info, StreamingContext context) | |||
{ | |||
Name = info.GetString("Name"); | |||
Age = (int)info.GetInt64("Age"); | |||
Names = | |||
((List<object>)info.GetValue("Names", typeof(List<object>))) | |||
Names = info.GetValue<List<object>>("Names") |
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 this change intended? The previous code looks rather proper.
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 thought info.GetValue<List<object>>("Names")
is better than ((List<object>)info.GetValue("Names", typeof(List<object>)))
. Do you think previous one is better?
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 thought it is not like better or worse, but just it's unlikely to be compiled. If the type checker passes I don't mind then.
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.
To elaborate, the changed version looks like trying to putting a List<object>
value into List<string>
property, which C#'s type checker does not allow AFAIK.
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.
There is more code below:
.Select(s => (string)s)
.ToList();
So I think it will pass the type checker.
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.
Ah, make sense.
…nuget ci(gh-actions): publish 'Libplanet.Explorer' to NuGet
…e-avatar Implement CreateAvatar2
…netarium#159) * INTERNAL: update configmap-versions.yaml * INTERNAL: update kustomization.yaml
BigInteger
toToBencodexValue
.