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

(refactor) Use Java 8 Streams, not Guava #330

Merged
merged 1 commit into from Dec 8, 2016

Conversation

JordanMartinez
Copy link
Contributor

No description provided.

@JordanMartinez
Copy link
Contributor Author

Note: There is still one section of guava code because I wasn't sure how to implement the method correctly:

private static <T> Set<T> combine(T input,
                                  Collection<Function<T, Set<T>>> functions) {
    return FluentIterable.from(functions)
        .transformAndConcat((f) -> f.apply(input))
        .toSet();
}

@brcolow
Copy link
Collaborator

brcolow commented Dec 4, 2016

Hmm..I am not too familiar with the Guava libraries and FluentIterable in particular.

But maybe something like this?

import java.util.Collection;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

    private static <T> Set<T> combine(T input, Collection<Function<T, Set<T>>> functions) {
        return functions.stream().map(function ->  function.apply(input)).flatMap(Collection::stream)
                .collect(Collectors.toSet());
    }

@JordanMartinez
Copy link
Contributor Author

Your code is correct, but the issue is that Collectors.toSet() does not care about the returned order of items in the returned set. Thus, the test that checks this method (assertThat(results, contains(label0, label1, ... button0, button1)) fails because contains checks order as well as contents. This can be resolved if containsInAnyOrder was used instead, but is that desired? Is order important?

@brcolow
Copy link
Collaborator

brcolow commented Dec 6, 2016

Hmmm...are the "leaf-nodes" of the scene-graph in a well defined order? If so, then I guess order does matter and we could add a sort call. If the "leaf-nodes" of the scene-graph are not in a well-defined order, then containsInAnyOrder is more appropriate.

@brcolow
Copy link
Collaborator

brcolow commented Dec 6, 2016

What I mean is, are the circled nodes in a well-defined order?

sgtree

@JordanMartinez
Copy link
Contributor Author

Yes, they seem to be in a well-defined order.

This is what the FXML looks like:

<StackPane maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="400.0" prefWidth="200.0" xmlns="http://javafx.com/javafx/8" xmlns:fx="http://javafx.com/fxml/1">
   <children>
      <VBox prefHeight="200.0" prefWidth="100.0">
         <children>
            <HBox fx:id="labels" spacing="20.0">
               <children>
                  <Label fx:id="label0" text="0" />
                  <Label fx:id="label1" text="1" />
                  <Label fx:id="label2" text="2" />
               </children>
               <padding>
                  <Insets bottom="10.0" left="10.0" right="10.0" top="10.0" />
               </padding>
            </HBox>
            <HBox fx:id="buttons" spacing="10.0">
               <padding>
                  <Insets bottom="10.0" left="10.0" right="10.0" top="10.0" />
               </padding>
               <children>
                  <Button fx:id="button0" mnemonicParsing="false" text="0" />
                  <Button fx:id="button1" mnemonicParsing="false" text="1" />
                  <Button fx:id="button2" mnemonicParsing="false" text="2" />
               </children>
            </HBox>

@brcolow
Copy link
Collaborator

brcolow commented Dec 8, 2016

@JordanMartinez
Copy link
Contributor Author

Still failed... :-/

I'll have to look into it another time. It's getting late.

@JordanMartinez
Copy link
Contributor Author

Fixed it. It works if I use a LinkedHashSet instead of a HashSet

@brcolow brcolow merged commit f134d8e into TestFX:master Dec 8, 2016
@brcolow
Copy link
Collaborator

brcolow commented Dec 8, 2016

Thanks! Merged.

@JordanMartinez JordanMartinez deleted the useJavaStreams branch December 8, 2016 20:52
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.

None yet

2 participants