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

Time.since relies on removed Signal.count #9

Closed
jwmerrill opened this Issue Nov 22, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Nov 22, 2014

Contributor

Here's my crack at a pure Elm implementation without count, but I haven't been able to test this yet because of some other issues getting running in the new world:

since : Time -> Signal a -> Signal Bool
since t s =
  let
    start = map (always 1) s
    stop = map (always -1) (delay t s)
    delaydiff = foldp (+) 0 (merge start stop)
  in
    map ((/=) 0) delaydiff
  • Edit, changed (==) to (/=) to get parity correct.
Contributor

jwmerrill commented Nov 22, 2014

Here's my crack at a pure Elm implementation without count, but I haven't been able to test this yet because of some other issues getting running in the new world:

since : Time -> Signal a -> Signal Bool
since t s =
  let
    start = map (always 1) s
    stop = map (always -1) (delay t s)
    delaydiff = foldp (+) 0 (merge start stop)
  in
    map ((/=) 0) delaydiff
  • Edit, changed (==) to (/=) to get parity correct.
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Nov 24, 2014

Member

This fix looks good to me. Can you confirm @jcollard?

Member

evancz commented Nov 24, 2014

This fix looks good to me. Can you confirm @jcollard?

@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Nov 24, 2014

Contributor

I made some comments on why I don't yet understand how to test this at the end of this e-mail thread: https://groups.google.com/d/msg/elm-discuss/Bs4nsLVZAXM/1drQHhqYccMJ

Contributor

jwmerrill commented Nov 24, 2014

I made some comments on why I don't yet understand how to test this at the end of this e-mail thread: https://groups.google.com/d/msg/elm-discuss/Bs4nsLVZAXM/1drQHhqYccMJ

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 24, 2014

Contributor

This implementation is close. At the end you should be checking (/=). I think the following version is more readable:

since : Time -> Signal a -> Signal Bool
since t s =
  let
    start = Signal.map (always True) s
    stop = Signal.map (always False) (delay t s)
  in Signal.foldp (\ n _ -> n) False (Signal.merge start stop)

Also, I'll make a reply to your message on how I go about testing changes so everyone can see.

Contributor

jcollard commented Nov 24, 2014

This implementation is close. At the end you should be checking (/=). I think the following version is more readable:

since : Time -> Signal a -> Signal Bool
since t s =
  let
    start = Signal.map (always True) s
    stop = Signal.map (always False) (delay t s)
  in Signal.foldp (\ n _ -> n) False (Signal.merge start stop)

Also, I'll make a reply to your message on how I go about testing changes so everyone can see.

@jcollard jcollard referenced this issue Nov 24, 2014

Closed

Add Time.since #16

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 24, 2014

Contributor

Even more readable:

since : Time -> Signal a -> Signal Bool
since t s =
  let initial = Signal.constant False
      start = Signal.map (always True) s
      stop = Signal.map (always False) (delay t s)
  in Signal.mergeMany [initial, start, stop]
Contributor

jcollard commented Nov 24, 2014

Even more readable:

since : Time -> Signal a -> Signal Bool
since t s =
  let initial = Signal.constant False
      start = Signal.map (always True) s
      stop = Signal.map (always False) (delay t s)
  in Signal.mergeMany [initial, start, stop]
@jwmerrill

This comment has been minimized.

Show comment
Hide comment
@jwmerrill

jwmerrill Nov 24, 2014

Contributor

You're right about the parity of mine being reversed. I'm going to go ahead and edit it to be correct.

Your version doesn't have exactly the same behavior as the existing since. For example, if I ask for since second Mouse.clicks and then click my mouse at half-second intervals, your version will alternate between True and False, but the existing version will never become True until I stop clicking at half second intervals.

Here's an Elm 0.13 program that compares the 3 implementations:

import Mouse
import Signal

joesince t s =
  let initial = Signal.constant False
      start = Signal.lift (always True) s
      stop = Signal.lift (always False) (delay t s)
  in Signal.merges [initial, start, stop]

jasonsince t s = 
  let
    start = Signal.lift (always 1) s
    stop = Signal.lift (always -1) (delay t s)
    delaydiff = foldp (+) 0 (merge start stop)
  in
    lift ((/=) 0) delaydiff

main = asText <~ Signal.lift3 (,,)
    (since second Mouse.clicks)
    (joesince second Mouse.clicks)
    (jasonsince second Mouse.clicks)

Try it out, and see what happens when you click your mouse regularly with a period of less than one second.

Contributor

jwmerrill commented Nov 24, 2014

You're right about the parity of mine being reversed. I'm going to go ahead and edit it to be correct.

Your version doesn't have exactly the same behavior as the existing since. For example, if I ask for since second Mouse.clicks and then click my mouse at half-second intervals, your version will alternate between True and False, but the existing version will never become True until I stop clicking at half second intervals.

Here's an Elm 0.13 program that compares the 3 implementations:

import Mouse
import Signal

joesince t s =
  let initial = Signal.constant False
      start = Signal.lift (always True) s
      stop = Signal.lift (always False) (delay t s)
  in Signal.merges [initial, start, stop]

jasonsince t s = 
  let
    start = Signal.lift (always 1) s
    stop = Signal.lift (always -1) (delay t s)
    delaydiff = foldp (+) 0 (merge start stop)
  in
    lift ((/=) 0) delaydiff

main = asText <~ Signal.lift3 (,,)
    (since second Mouse.clicks)
    (joesince second Mouse.clicks)
    (jasonsince second Mouse.clicks)

Try it out, and see what happens when you click your mouse regularly with a period of less than one second.

@jcollard

This comment has been minimized.

Show comment
Hide comment
@jcollard

jcollard Nov 24, 2014

Contributor

You're correct. Thank you for pointing it out.

Contributor

jcollard commented Nov 24, 2014

You're correct. Thank you for pointing it out.

@evancz evancz closed this in c660305 Nov 24, 2014

evancz pushed a commit that referenced this issue Nov 24, 2014

Merge pull request #18 from jwmerrill/pure-elm-since
Replace Native `since` with a pure elm version. Fixes #9.
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Nov 24, 2014

Member

Thanks guys! :D

Member

evancz commented Nov 24, 2014

Thanks guys! :D

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