-
Notifications
You must be signed in to change notification settings - Fork 165
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
#1335: NumerEnvelope wrap function restored #1488
Conversation
public final double doubleValue() { | ||
return new Unchecked<>(this.dnum).value(); | ||
public float floatValue() { | ||
return origin.floatValue(); |
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.
@ruca905 Please qualify field access with this.
@@ -65,49 +65,26 @@ public NumberOf(final String txt) { | |||
this(new TextOf(txt)); | |||
} | |||
|
|||
public NumberOf(final Scalar<Long> lnm, final Scalar<Integer> inm, |
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.
@ruca905 Could you place each argument on separate line?
@@ -235,7 +235,7 @@ public MaxOf(final Float... src) { | |||
} | |||
} | |||
return max; | |||
}); | |||
})); |
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.
@ruca905 It would be match better not to cluster parenthesis this way
@ruca905 The build is failing, I believe you have many qulice complains as well. Also the topic of PR is not very clear, and the body lacks a link to the issue |
@ruca905 Any update on this? |
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.
@ruca905 here are some last changes to finish this PR. Ping me when it's done :)
} | ||
|
||
@Override | ||
public final int intValue() { | ||
return new Unchecked<>(this.inum).value(); | ||
public Double value() throws Exception { |
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.
@ruca905 please remove the implementation of Scalar<Double>
, it's out of scope of NumberEnvelope
.
this.fnum = fnm; | ||
this.dnum = dnm; | ||
public NumberEnvelope(final Number number) { | ||
this.origin = number; |
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.
@ruca905 please also delegate byteValue, shortValue, toString, equals and hashCode
final Scalar<Integer> inm, | ||
final Scalar<Float> fnm, | ||
final Scalar<Double> dnm) { | ||
this(dnm); |
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.
@ruca905 I think there was some kind of confusion about what NumberOf
and Sealed
were about… so, now that it's clearer to me, I think we should merge Sealed
and NumberOf
into NumberOf
, for this:
- we move the old code based on multiple scalars from
NumberEnvelope
toNumberOf
- we move the constructors of
Sealed
toNumberOf
- we keep the text/string based constructor here and delegate to the scalar-based ones
- we delete
Sealed
@@ -67,7 +63,7 @@ public void readFromGzipInput() throws Exception { | |||
).affirm(); | |||
} | |||
|
|||
@Test(expected = EOFException.class) | |||
@Test(expected = UncheckedIOException.class) |
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.
@ruca905 please revert all changes from this file
@@ -86,7 +81,7 @@ public void writeToGzipOutput() throws Exception { | |||
).affirm(); | |||
} | |||
|
|||
@Test(expected = IOException.class) | |||
@Test(expected = UncheckedIOException.class) |
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.
@ruca905 please revert all changes from this file
@sereshqua/z please review this job completed by @andreoss/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
Code review was too long (66 days), architects (@victornoel) were penalized, see §55 |
@ruca905 please make sure that all problems addressed by code reviewers would be fixed by you in next PRs, thanks |
@0crat quality acceptable |
#1335