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

List does not throw runtime exceptions #90

Merged
merged 6 commits into from Jan 11, 2015

Conversation

Projects
None yet
2 participants
@laszlopandy
Contributor

laszlopandy commented Jan 10, 2015

  • Changed type List.head : List a -> Maybe a
  • Deleted List.tail in favour of List.drop 1
  • Deleted List.foldl1, List.foldr1, List.scanl1
  • Added List.uncons
  • Changed List.maximum and List.minimum to take default values
Show outdated Hide outdated src/Graphics/Element.elm
DLeft -> newFlow (List.sum ws) maxH
DRight -> newFlow (List.sum ws) maxH
DIn -> newFlow maxW maxH
DOut -> newFlow maxW maxH

This comment has been minimized.

@evancz

evancz Jan 10, 2015

Member

This code does extra traversals compared to the old way. I don't know how big a deal that is, but I wrote it the old way with this in mind.

@evancz

evancz Jan 10, 2015

Member

This code does extra traversals compared to the old way. I don't know how big a deal that is, but I wrote it the old way with this in mind.

Show outdated Hide outdated src/Signal.elm
@@ -148,7 +149,9 @@ update wins, just like with `merge`.
-}
mergeMany : List (Signal a) -> Signal a
mergeMany signals =
List.foldr1 merge signals
case signals of
s :: rest -> List.foldr merge s rest

This comment has been minimized.

@evancz

evancz Jan 10, 2015

Member

I think this is buggy. s is the leftmost thing, but it is used as the rightmost. I think foldr needs the last element.

@evancz

evancz Jan 10, 2015

Member

I think this is buggy. s is the leftmost thing, but it is used as the rightmost. I think foldr needs the last element.

This comment has been minimized.

@laszlopandy

laszlopandy Jan 10, 2015

Contributor

I did a reverse and foldl. I'm pretty sure this is equivalent to foldr, but please check if over again.

@laszlopandy

laszlopandy Jan 10, 2015

Contributor

I did a reverse and foldl. I'm pretty sure this is equivalent to foldr, but please check if over again.

Show outdated Hide outdated src/List.elm
minimum : List comparable -> comparable
minimum = foldl1 min
minimum : comparable -> List comparable -> comparable
minimum def list = foldl min def list

This comment has been minimized.

@evancz

evancz Jan 10, 2015

Member

I think it is often hard to pick a minimum value. I'm imagining people writing minimum (maximum 0 xs) xs.

Why did you go for this instead of returning a maybe?

@evancz

evancz Jan 10, 2015

Member

I think it is often hard to pick a minimum value. I'm imagining people writing minimum (maximum 0 xs) xs.

Why did you go for this instead of returning a maybe?

This comment has been minimized.

@evancz

evancz Jan 10, 2015

Member

I am having a hard time evaluating this change without any examples as data.

@evancz

evancz Jan 10, 2015

Member

I am having a hard time evaluating this change without any examples as data.

This comment has been minimized.

@laszlopandy

laszlopandy Jan 10, 2015

Contributor

Yeah I just realized today that there is a difference between a default
value (which will be ignored if the list is non-empty) and a base value for
foldl.

So I will change it back to maybe.

On Saturday, January 10, 2015, Evan Czaplicki notifications@github.com
wrote:

In src/List.elm
https://github.com/elm-lang/core/pull/90#discussion-diff-22759619:

-}
-minimum : List comparable -> comparable
-minimum = foldl1 min
+minimum : comparable -> List comparable -> comparable
+minimum def list = foldl min def list

I am having a hard time evaluating this change without any examples as
data.


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/pull/90/files#r22759619.

@laszlopandy

laszlopandy Jan 10, 2015

Contributor

Yeah I just realized today that there is a difference between a default
value (which will be ignored if the list is non-empty) and a base value for
foldl.

So I will change it back to maybe.

On Saturday, January 10, 2015, Evan Czaplicki notifications@github.com
wrote:

In src/List.elm
https://github.com/elm-lang/core/pull/90#discussion-diff-22759619:

-}
-minimum : List comparable -> comparable
-minimum = foldl1 min
+minimum : comparable -> List comparable -> comparable
+minimum def list = foldl min def list

I am having a hard time evaluating this change without any examples as
data.


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/pull/90/files#r22759619.

@evancz evancz merged commit 708bd58 into elm:master Jan 11, 2015

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment