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

Add more documentation to EventStream ? #34

Open
JordanMartinez opened this issue Nov 11, 2015 · 32 comments
Open

Add more documentation to EventStream ? #34

JordanMartinez opened this issue Nov 11, 2015 · 32 comments

Comments

@JordanMartinez
Copy link
Contributor

I wanted to add more documentation to EventStream that explains a few things:

  • The lifecycle of an EventStream object: creation, subscribe, unsubscribe
  • Subscription and how and why to use it to prevent memory leaks
  • Different ways to create an EventStream object
  • The different composition methods that are grouped together by common categories:
    • time-related
    • change-related
    • suspendable-related, etc.

When I first looked at this library, the Readme file helped show the usefulness of EventStream objects in general. Since EventStream, as opposed to Val/Var or LiveList, was demonstrated in the Readme file, I looked at that first. However, I didn't immediately know where to go from there, much less how to use the code in my program. My guess is that others who wish to know more about how to use this library would follow a similar path, hence why I think the documentation should be added to EventStream.

@JordanMartinez
Copy link
Contributor Author

Here are my ideas for better documentation:

  • In the javadoc of EventStreams & EventStream, better explain the different ways an EventStream object is created by using the following approach:
/*
ConditionOn Explanation:

EventStream<X> A
EventStream<X> B = A.conditionOn(Property)
*/

// Returns B. When A emits an event, B also emits that event if Property evaluates to true. 
// The property is analogous to creating a gate that is either open (evaluates to true) 
//   or closed (evaluates to false).

/*
Combine Explanation

EventStream<A> 1
EventStream<B> 2
EventStream<C> 3
. . .
EventStream<?> N
EventStream<Tuple#<A, B, C, ..., ?> X = EventStreams.combine(1, 2, 3, ..., N)
*/

// Returns X. X does not emit its combined event until 1, 2, 3, ..., and N have all emitted at least one event. 
// Once X emits its first event, it will emit a new event every time 1, 2, 3, ..., or N emit a new event.
  • In a wikipage, explain
    • The basic parts of an EventStream's lifecycle: creation, (optional) composition, subscribe, unsubscribe
    • Subscription, how to use it to prevent memory leaks, and (often used by Tomas) how to funnel all subscriptions into one Subscription object for easy unsubscribing.
    • A digram that groups different EventStream objects together and shows how one group can lead into another by some method. This would be a helpful diagram to consider when trying to understand how to best compose an EventStream for some situation.
    • Val is essentially a ReadOnlyProperty object whereas Var is a ReadAndWriteProperty. Also, Val/Var can create their own EventStream objects. Also highlights the issue of ObjectProperty binding to a Val/Var and workaround for that (example comes from a ScrollBar's h/vvalueProperty unable to bind to a Val/Var).

@JordanMartinez
Copy link
Contributor Author

I've written some pages for the wiki that should help guide someone who is unfamiliar with this library.

@TomasMikula
Copy link
Owner

Great, many thanks for volunteering to do this!

@TomasMikula
Copy link
Owner

Somewhere I would like to have the advice to minimize the number of calls that return a Subscription, because one then needs to think about when to unsubscribe. One often can go a long way with just composing streams, which stay lazy.

@JordanMartinez
Copy link
Contributor Author

I want to write another page that focuses on composing EventStreams, so it'd probably be best if that content went into that page.

@JordanMartinez
Copy link
Contributor Author

I wrote another page that includes the Subscription minimizing.

Beyond that, there's the EventStream ecosystem. I think a good way to show the relationships between categories would be to map the library using Freeplane and screenshotting it.

@JordanMartinez
Copy link
Contributor Author

I've uploaded a photo in the wiki that shows the idea I had. I'm sure that photo could be refined by better categorizing the methods from EventStream. However, that's where my knowledge of this library starts to get fuzzy pretty fast. Additionally, I forgot to add EventStreams#never to the creation part.

@TomasMikula
Copy link
Owner

Thanks! Regarding your composition example, you split combo into 6 streams that you then almost immediately merge back together. You can achieve the same more simply:

EventStream<Paint> fillChanger = combo.map(t -> t.map((hover, ctrl, shift, transfer) -> {
    if(!hover && !ctrl && !shift && !transfer) return Color.RED;
    else if(...) ...;
    ...
    else ...;
}));

You also seem to be covering only 6 cases out of 16 possible.

Having to merge initialValue is not very nice, but I don't have a good alternative now. Things that would come in handy here would be accumulate0 or prepend(T t).

@JordanMartinez
Copy link
Contributor Author

Thanks! Regarding your composition example, you split combo into 6 streams that you then almost immediately merge back together.

Lol... Yeah, I figured my approach wasn't very efficient. :-) My goal was just to make sure people got the idea of starting from the final EventStream and then working backwards.

You also seem to be covering only 6 cases out of 16 possible.

You mean the other methods in the EventStream class?

Having to merge initialValue is not very nice

Aye... But that's the workaround I came up with in my program's code.

Things that would come in handy here would be accumulate0 or prepend(T t).

Aye, those would be.

@TomasMikula
Copy link
Owner

You mean the other methods in the EventStream class?

No. You have 4 boolean variables. That gives 24=16 combinations. What color corresponds to, e.g., YNNY?

@JordanMartinez
Copy link
Contributor Author

No. You have 4 boolean variables. That gives 24=16 combinations. What color corresponds to, e.g., YNNY?

Ah... I was just trying to come up with a simple example that uses multiple inputs. I also didn't want to have to create a table that showed all 16 combinations, just enough to get the point across.

@TomasMikula
Copy link
Owner

Doesn't the current version mean that for the undefined combinations, the color depends on the previous state, and thus is not unique?

You don't necessarily have to list all 16 rows. A row

N * * *

covers 8 cases and translates to an if that tests only 1 variable.

@JordanMartinez
Copy link
Contributor Author

Doesn't the current version mean that for the undefined combinations, the color depends on the previous state, and thus is not unique?

Yes.

You don't necessarily have to list all 16 rows. A row N * * * covers 8 cases and translates to an if that tests only 1 variable.

I think I understand what you're staying, but not entirely.

@TomasMikula
Copy link
Owner

All I'm saying is that these 8 rows

a b c d color
N N N N Red
N N N Y Red
N N Y N Red
N N Y Y Red
N Y N N Red
N Y N Y Red
N Y Y N Red
N Y Y Y Red

that would directly translate to 8 ifs

if(!a && !b && !c && !d) return RED;
else if(!a && !b && !c && d) return RED;
else if(!a && !b && c && !d) return RED;
...
else if(!a && b && c && d) return RED;

can be captured as

a b c d color
N * * * Red

which translates to

if(!a) return RED;

@JordanMartinez
Copy link
Contributor Author

So, I think I finally have the words to rephrase what you're saying here :-D (I didn't understand you well enough the first night I read this to be able to restate what you said, but looking at it now, I believe I can).

The mapping example I gave in the wiki page allows for 16 total combinations. However, my example only addresses 6 of those possible combinations. Thus, there are 10 situations where an Event is emitted, but no code is run.
In order to include the remaining 10 situations without writing any additional code (specifically, code that evaluates every situation of the remaining 10), I should remove some of the unneeded boolean evaluations. So, instead of writing if (a && b && c && d), if (a && ... && !d), and the other variants of this statement that change b, c, and d, I should just write if (a) because it takes into account the remaining 7 variants without needing to evaluate them. Moreover, all 8 combinations can be evaluated by one if statement as opposed to four of them.

So, if I'm understanding you correctly, yes you are right. I could and probably should have done that for neater code.

@JordanMartinez
Copy link
Contributor Author

Getting back to this topic...

  • I'm realizing that much of the documentation for the EventStream objects can be derived from their actual tests.
  • I also think there should be another topic in the wiki that addresses
    • how to debug an EventStream flow
    • how to write your own EventStream class

On a different note, I've been reading through the book, Java Testing with Spock, because I've heard about tests before but now I actually understand why they're worth writing. So, would the tests themselves also be included in the scope of this issue? And if so, what about using Spock instead of JUnit?

@TomasMikula
Copy link
Owner

What do you mean by

would the tests themselves also be included in the scope of this issue?

Do you mean writing more tests, or referring to tests as documentation?

I'm not familiar with Spock. What benefits would it bring?

@JordanMartinez
Copy link
Contributor Author

I mean should the tests be updated to include more comments throughout the test itself?

Most of the tests that have been written only have code and not comments explaining what is expected at various parts throughout the test or what each object within the test does. This is part of the reason why I'm also bringing up Spock.

Whereas a JUnit test is shown as

@Test
void someTest() {
  // set up code
  // some other code
  // more set up code
  // does anyone really know what's going on here????
  // ...
  // oh look! Some "assertEquals" statements
}

a Spock test would look like:

def "when user presses shift, Rectangle's fill color will change to Red"() {
// this is the set up part of the code
    given: "a rectangle"
        Rectangle rectangle = new Rectangle()

    and: "a property that restricts changes when false"
        Var<Boolean> allowChange = Var.simpleVar(true)

    and: "an EventStream that emits an event when shift is pressed & allowChange is true"
        EventStream<KeyEvent> keyPressed = EventStreams
            .eventsOf(rectangle, KeyEvent.KEY_PRESSED)
            .filter(it.code == KeyCode.SHIFT)
            .conditionOn(allowChange)

    and: "a subscriber to keyPressed that changes rectangle's fill color"
        keyPressed.subscribe( { rectangle.setFill(Color.RED) })

    and: "a fake KeyEvent"
        KeyEvent event = Stub(KeyEvent)
        event.getTarget() >> rectangle
        event.isShiftDown() >> true

// this is the trigger or test case
    when: "the user presses shift"
        Event.fireEvent(event)

// this evaluates whether the test passes or fails
    then: "the rectangle's fill changes to Red"
        rectangle.fillProperty().get() == Color.RED
}

Spock brings the following features:

  • readability
  • enforces good test structure (the test is broken up into understandable parts)
  • allows Stubbing and Mocking
    • in the example above, event is pre-programmed to return the given values when those methods are called
  • makes parameterization tests much easier and more readable

For example, parameterized tests look like:

def "some parameterized test"() {
    given: "A rectangle with a starting color and layoutY value"
        Rectangle rectangle = new Rectangle()
        rectangle.setFill(color)
        rectangle.setLayoutY(position)

    when: "randomize is called"
        // let's say randomize increments position by 2 and 
        //   changes the fill color to the next color in the rainbow

    then: "Rectangle has the correct color and layoutY value"
        rectangle.getFill() == newColor
        rectangle.getLayoutY() == newPosition

    where: "inputs and outputs are"
        // normally, this table would line up vertically properly, 
        //   but that's hard to do on here.
        color | position || newColor | newPosition
        Color.RED | 0   || Color.ORANGE | 2
        Color.YELLOW | 2 || Color.GREEN | 4
        // etc.

@TomasMikula
Copy link
Owner

I see. If there's a reasonable Java version, I'm not opposed to adding Spock as a test framework. Though not instead of JUnit, but alongside it.

@JordanMartinez
Copy link
Contributor Author

If there's a reasonable Java version, I'm not opposed to adding Spock as a test framework.

It uses groovy, so I'm guessing that's a no ;-) To use it, one would need to add a dependency to the groovy language.

@TomasMikula
Copy link
Owner

The dependency would only be for tests, not for the production release, so I don't mind that. But I don't think I want to drag another language into RichTextFX. I wouldn't mind using some other BDD framework that works in pure Java. Though I don't have experience with or recommendation for any.

@JordanMartinez
Copy link
Contributor Author

Well... I think the framework is helpful for various things, but that doesn't mean it needs to be used here. I was thinking more in terms of clarity: if we want to make the tests more understandable, then Spock would do a good job at that, but the same purpose could be achieved if the JUnit tests already written are updated with more commentary.

@JordanMartinez
Copy link
Contributor Author

I wouldn't mind using some other BDD framework that works in pure Java.

What functionality doesn't exist in JUnit that you would want to use here?

@TomasMikula
Copy link
Owner

if we want to make the tests more understandable, then Spock would do a good job at that

It's a double-edged sword, since you limit the audience/writers to people willing to read/write Groovy.

What functionality doesn't exist in JUnit that you would want to use here?

The question is, is there a pure Java testing framework that has the functionality similar to Spock's, that you would like to use?

@JordanMartinez
Copy link
Contributor Author

It's a double-edged sword, since you limit the audience/writers to people willing to read/write Groovy.

Good point.

The question is, is there a pure Java testing framework that has the functionality similar to Spock's, that you would like to use?

No, JUnit is fine. My goal here is just to make the tests clearer. That can be done easily with some commentary. I just thought I'd mention Spock because I think it does this more clearly than JUnit.

@JordanMartinez
Copy link
Contributor Author

I just noticed that you moved the 'Error Handling' part in the home page's wiki to Obsolete.

With that being so, what does a developer use to debug an EventStream? I've understood the hook EventStream to serve that purpose, but I wasn't sure if there was something else in addition to that.

@TomasMikula
Copy link
Owner

Yes, hook is for that purpose and is not obsolete. However, the mechanism described in that Wiki page was removed.

@JordanMartinez
Copy link
Contributor Author

I've updated the EventStream Intro page to include a small section about debugging with hook at its end.

@TomasMikula
Copy link
Owner

👍

@JordanMartinez
Copy link
Contributor Author

I added a page on State Machines to the wiki as your blog post is not something most people will see unless they know where to look and the class itself might get lost among all the other classes. This makes it much easier to find and see its usages.

@TomasMikula
Copy link
Owner

Thanks for putting some effort into documentation! 👍

@JordanMartinez
Copy link
Contributor Author

The only thing left to do in this issue is document the accumulative-related EventStreams and StateMachine, which should be added to EventStream's interface javadoc, stating that one should use it when the composition methods given in the interface are not flexible enough to account for the need.

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

No branches or pull requests

2 participants