Skip to content
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

testing case: merge on atomic update #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iofjuupasli
Copy link

AssertionError: [ 2, 6 ] deepEqual [ 2, 5, 4, 6 ]

result.push(d());
});
a(2);
assert.deepEqual(result, [2, 5, 4, 6]);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I suppose that events will be emitted in the same order as streams provided to flyd.merge.
Should that be like that? Or maybe opposite order, because of curry-in-mind API? Or maybe it should depends on order of stream declarations?

@paldepind
Copy link
Owner

This is intended behavior. When a changes in your test case b and c update at the same time. But d cannot emit two events at the same time so it has to drop one of them.

From the section about atomic updates:

Flyd guarantees that when a single value enters the system every stream will only be updated once, along with their dependencies in their most recent state.

If the merged stream turned one entering event into two events the above would not be true and atomic updates wouldn't work.

Does the above explanation make sense to you? Do you have a use case for the suggested behavior?

@iofjuupasli
Copy link
Author

This is intended behavior.

Surprising. This behaviour seems broken

Flyd guarantees that when a single value enters the system every stream will only be updated once, along with their dependencies in their most recent state.

I think it's wrong preposition. Case approves that.

Atomic updates should guarantee that combine emits only once if both (or more) deps updated at once.

Also right now it isn't true for async streams. I can emit one event, and then asynchronously emit more than one event in another stream.

So, I think you should alter section in readme instead of introducing unexpected behaviour based on fallacy 'atomic equals one'.

I think it's possible to have two event on one stream at one time. Most probably it's hard to implement in general.

Do you have a use case for the suggested behavior?

No. But I can imagine that case (logging, show some messages to user (achievements)).

@paldepind
Copy link
Owner

Surprising. This behaviour seems broken

Atomic updates means that when a stream changes all it's dependencies changes at the exact same time. Thus d can only change once when both of it's dependencies changes at the same time. This is a consequence of atomic updates.

Case approves that.

What case?

Also right now it isn't true for async streams. I can emit one event, and then asynchronously emit more than one event in another stream.

Please explain this?

So, I think you should alter section in readme instead of introducing unexpected behaviour based on fallacy 'atomic equals one'.

This should definitely be documented. But atomic doesn't equal one. In this context it means 'at once'. All updates happens 'at once'. Two changes from the same stream can't happen 'at once' and thus the behavior you request would go against the idea of atomic updates.

I think it's possible to have two event on one stream at one time.

How so? Imagine that a stream is like a timeline with changes/events on it. You can have two changes/events infinitely close to each other but you can not have two changes that happens in the exact same moment.

@iofjuupasli
Copy link
Author

What case?

Test case.

Please explain this?

Flyd guarantees that when a single value enters the system every stream will only be updated once, along with their dependencies in their most recent state.

var a = stream()
var b = stream([a], function(self){
  self(1)
  setTimeout(function(){self(2)}, 0);
})

For every emitted a there will be two b events.

you can not have two changes that happens in the exact same moment

Why?

var a = stream();
a([new Event(1), new Event(2)])

I have abstracted it on my side, but that is really two events at once.

@paldepind
Copy link
Owner

Test case.

A test case doesn't define correct behavior. I could easily create a passing test case that validates the current behavior.

For every emitted a there will be two b events.

Two event's enter the system here. One when you invoke a and one when you asynchronously invoke self.

Why?

Because one property cannot have two different values at the same time. d can't be both 2 and 5 at the same time.

I have abstracted it on my side, but that is really two events at once.

Yes. You can use a stream of arrays to represent a stream that contains multiple values at the same time. That is not a problem.

@iofjuupasli
Copy link
Author

property cannot have two different values at the same time

Right. But stream can be emitter of two different value at the same time.

var weaponLeftHand = stream(1);
var weaponRightHand = stream(5);

var playerAction = stream()

var damageLeftHand = stream([playerAction], function () {
    if (playerAction() === 'attack') {
        return weaponLeftHand();
    }
});

var damageRightHand = stream([playerAction], function () {
    if (playerAction() === 'attack' && skills().indexOf('superpower') !== -1) {
        return weaponRightHand();
    }
});

var opponentResist = stream(3);

var damage = filter(function (damage) {
    return damage > opponentResist();
}, merge([damageLeftHand, damageRightHand]));

var opponentHeals = scan(function (hp, damage) {
    // this called twice because damage is stream
    return hp - damage;
}, damage);
// but emmits only one event because this is property

var opponentHealsGui = map(function (hp) {
    return '<div>' + hp + '</div>';
}, opponentHeals);

I know, there is no stream/property in flyd.
But ideally I would expect behavior described in example.
I know how to implement example with array of damages.
But I don't like using array because it adds one more dimension to program.

a)
----1------
-------1---
----1--1---

b)
----1------
----1------
---[1------
    1]

a is same as b except time of emitting. From 3rd (merge) stream I want to just emit events from both 1st and 2nd stream. Depending on time I don't want to have different ways to handle events. Sudden change of paradigm.

The only problem with two events at time is state that depends on one last value or on order of events. Because we can't separate this two events in a time and say who was the last one and in which order apply events to property.

I think that it's possible in theory. But practically implementing that will be not so simple, and using very rare and limited (associativity of operations on events).

I think current behavior should be mentioned in readme in merge and atomic sections.

@iofjuupasli iofjuupasli closed this Jun 8, 2015
@paldepind
Copy link
Owner

I know, there is no stream/property in flyd.

Streams are more like properties. They have a current value. In the test case in your PR what would the value of d be in the end? If two events were emitted at the exact same time it should somehow have two values at once?

This behavior is not something that I've invented. It might not be intuitive but when you consider what atomic updates mean I think it's the only thing that makes sense. Bacon doesn't have atomic updates for EventStreams. Elm drops values in merge exactly like Flyd does.

What about doing something like this:

var damageBothHands = stream([playerAction], function () {
    var damage = 0;
    if (playerAction() === 'attack') {
        damage += weaponLeftHand();
    }
    if (playerAction() === 'attack' && skills().indexOf('superpower') !== -1) {
        damage += weaponRightHand();
    }
    if (damage !== 0) return damage;
});

I understand that this is not as simple as if Flyd had the behavior you request. I'll think about this some more and see if I can think of a better solution.

I think current behavior should be mentioned in readme in merge and atomic sections.

I certainly agree and I will reopen this issue until that is done. I am sorry if this lack of documentation has been a hurdle for you.

@paldepind paldepind reopened this Jun 8, 2015
@paldepind
Copy link
Owner

@iofjuupasli What about a merge that always return changes in an array and a spread function that maps a function over items in a stream of arrays?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants