make sum(), min(), max() tolerate unstable first/rest #414

Merged
merged 1 commit into from Mar 20, 2014

Conversation

Projects
None yet
3 participants
Contributor

sgalles commented Feb 21, 2014

An humble contribution based on a what was recently done for reduce(), but this time for sum(), min() and max().
If this modification was not desirable please just ignore this pull request

@tombentley tombentley commented on the diff Mar 20, 2014

src/ceylon/language/max.ceylon
@@ -17,6 +17,8 @@ shared Absent|Value max<Value,Absent>(Iterable<Value,Absent> values)
return max;
}
else {
- return first;
+ "iterable must be empty"
+ assert (is Absent null);
@tombentley

tombentley Mar 20, 2014

Member

I'm not sure we really need this assert. Absent can only be Null or Nothing for the possibly-empty and non-empty cases. If it's non-empty (Nothing) we never get to the else block: If we get to the else block then Absent must be null Null.

gavinking added this to the 1.1 milestone Mar 20, 2014

gavinking self-assigned this Mar 20, 2014

Contributor

sgalles commented Mar 20, 2014

Actually I had shamelessly copied what @gavinking had done here bf20c0c
Indeed, it looks like a defensive assert. Maybe @gavinking could tell us if we must keep it or remove it.

Owner

gavinking commented Mar 20, 2014

Well in my code I had an issue where null was not necessarily a subtype of Absent, so to return null I had to do the assert first. Not sure if you have the same problem.

Contributor

sgalles commented Mar 20, 2014

ha, now it rings a bell, but I can't remember if I had this problem in my
code too.
I'm going to check tonight and I let you know.

On Thu, Mar 20, 2014 at 1:17 PM, Gavin King notifications@github.comwrote:

Well in my code I had an issue where null was not necessarily a subtype
of Absent, so to return null I had to do the assert first. Not sure if
you have the same problem.

Reply to this email directly or view it on GitHubhttps://github.com/ceylon/ceylon.language/pull/414#issuecomment-38160004
.

Contributor

sgalles commented Mar 20, 2014

Confirmed, it is the same problem.
We must keep the assert in min() and max()
FTR if I try to remove the assert :
returned expression must be assignable to return type of min: null is not assignable to Absent|Value

@tombentley tombentley added a commit that referenced this pull request Mar 20, 2014

@tombentley tombentley Merge pull request #414 from sgalles/unstableit
make sum(), min(), max() tolerate unstable first/rest
3935034

@tombentley tombentley merged commit 3935034 into ceylon:master Mar 20, 2014

Member

tombentley commented Mar 20, 2014

Thanks very much!

Contributor

sgalles commented Mar 20, 2014

Thanks for the merge !

Member

tombentley commented Mar 21, 2014

No, thank you. And sorry it took so long.

Contributor

sgalles commented Mar 21, 2014

No problem :)

On Fri, Mar 21, 2014 at 8:39 AM, Tom Bentley notifications@github.comwrote:

No, thank you. And sorry it took so long.

Reply to this email directly or view it on GitHubhttps://github.com/ceylon/ceylon.language/pull/414#issuecomment-38254353
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment