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

Improve collection types #523

Merged
merged 51 commits into from
Jan 16, 2020
Merged
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
04bf1bd
Migrate Triggerable.targets to Set
ggalmazor Dec 16, 2019
ed1bae4
Migrate TriggerableDag.triggerablesDAG to a LinkedHashSet
ggalmazor Dec 16, 2019
beb274b
Migrate TriggerableDag.triggerIndex to a map of sets
ggalmazor Dec 16, 2019
de6efcf
Migrate TriggerableDag.unorderedTriggers to a Set
ggalmazor Dec 16, 2019
883ec3b
Migrate to immutable and side-effect free code
ggalmazor Dec 17, 2019
9b0f5a5
Migrate to immutable and side-effect free code - Phase 2
ggalmazor Dec 17, 2019
4a75184
Migrate to immutable and side-effect free code - Phase 3
ggalmazor Dec 17, 2019
e671744
Migrate to immutable and side-effect free code - Phase 4
ggalmazor Dec 17, 2019
acd8d54
Simple rearrange
ggalmazor Dec 17, 2019
49a596b
Add TODO note
ggalmazor Dec 17, 2019
73e6364
Improve naming and extract methods to reduce duplication of concepts
ggalmazor Dec 17, 2019
eaa0f81
Extract inner loop in cascade outside of the main loop
ggalmazor Dec 17, 2019
1f7d147
Inline for more concise code
ggalmazor Dec 17, 2019
aaf6114
Separate computing the DAG from assigning it
ggalmazor Jan 8, 2020
0325420
Extract method that builds the DAG
ggalmazor Jan 8, 2020
d996178
Simplify assigning a new DAG
ggalmazor Jan 8, 2020
ccc0087
Pull up computing the edges to avoid passing through dependencies
ggalmazor Jan 8, 2020
a8b9c7a
Renaming and sorting methods and variables for improved reading
ggalmazor Jan 8, 2020
2f6ddf8
Make the dag building algorithm immutable
ggalmazor Jan 8, 2020
04a39f0
Remove unnecessary sorting of the nodes added to the DAG
ggalmazor Jan 8, 2020
6b7b52f
Simplify by adding the roots collection directly
ggalmazor Jan 8, 2020
fc112c7
Make member explicitly mutable
ggalmazor Dec 17, 2019
6157ffd
Improve names and context to understand what finalizing the DAG means
ggalmazor Dec 17, 2019
8eaaff1
Remove docblock comments
ggalmazor Dec 17, 2019
bffc1be
Rename members for more clarity
ggalmazor Dec 17, 2019
901156c
Reorder members
ggalmazor Dec 17, 2019
39d8cd0
Review dag.addTriggerable to provide up-to-date context and detail
ggalmazor Dec 17, 2019
0c012b3
Add docblock detailing side effects that might not be apparent
ggalmazor Dec 17, 2019
ebb51f5
Avoid confusing value reassignation dance
ggalmazor Dec 17, 2019
303e968
Remove unused imports
ggalmazor Dec 20, 2019
2bd11f6
Avoid hitting the main instance to get the dependant triggerables
ggalmazor Jan 8, 2020
1ddaed6
Inline methods only used once
ggalmazor Jan 8, 2020
d594ef4
Reuse the ref object
ggalmazor Jan 8, 2020
d28f98a
Remove redundant check
ggalmazor Jan 8, 2020
b4c83c2
Clean up code: naming, simplify, comments
ggalmazor Jan 8, 2020
47ee88e
Replace construction that's not compatible with Android API level 16
ggalmazor Jan 9, 2020
1e3cf39
Remove unused methods
ggalmazor Jan 9, 2020
86db7bf
Add comments explaining how the sets in the dag build get mutated
ggalmazor Jan 9, 2020
150a469
Add doc blocks for TriggerableDag members. Wrap at 80 chars
ggalmazor Jan 10, 2020
8c4ef1e
Make the DAG edge building algorithm easier to understand
ggalmazor Jan 10, 2020
11537bf
Remove redundant check
ggalmazor Jan 10, 2020
6e9aba9
Improve naming for less confusion
ggalmazor Jan 10, 2020
1f31f7d
Make the getDagEdges method non-static to avoid confusion by arg names
ggalmazor Jan 13, 2020
a942f47
Improve comment by using unambiguous language
ggalmazor Jan 13, 2020
3cfecc1
Change the collection type to set for conformity
ggalmazor Jan 13, 2020
f764f82
Add end-to-end test to verify that the DAG is sorted
ggalmazor Jan 13, 2020
0a25712
Revert getChidlrenOfReference method to the original implementation
ggalmazor Jan 13, 2020
ac646eb
Rename to avoid misguiding use of "children"
ggalmazor Jan 13, 2020
757ea20
Remove unnecessary comment
ggalmazor Jan 16, 2020
0fa1549
Add TO DO notes to remember code exploration insights and questions
ggalmazor Jan 16, 2020
a3b7a18
Make private all the protected members
ggalmazor Jan 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/main/java/org/javarosa/core/model/TriggerableDag.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.javarosa.core.model.condition.EvaluationContext;
import org.javarosa.core.model.condition.Recalculate;
import org.javarosa.core.model.condition.Triggerable;
import org.javarosa.core.model.instance.AbstractTreeElement;
import org.javarosa.core.model.instance.FormInstance;
import org.javarosa.core.model.instance.TreeElement;
import org.javarosa.core.model.instance.TreeReference;
Expand Down Expand Up @@ -344,26 +343,31 @@ private static List<QuickTriggerable[]> getDagEdges(Set<QuickTriggerable> vertic
}

private static Set<QuickTriggerable> buildDag(Set<QuickTriggerable> vertices, List<QuickTriggerable[]> edges) {
// The dag and the set of remaining vertices will be mutated
// inside the while loop's block
Set<QuickTriggerable> dag = new LinkedHashSet<>();

Set<QuickTriggerable> remainingVertices = new HashSet<>(vertices);

// The set of remaining edges will be replaced inside
// the while loop's block
Set<QuickTriggerable[]> remainingEdges = new HashSet<>(edges);

while (remainingVertices.size() > 0) {
// Compute the set of roots (nodes that don't show up
// as edge targets) with the remaining vertices
Set<QuickTriggerable> roots = new HashSet<>(remainingVertices);
for (QuickTriggerable[] edge : remainingEdges)
roots.remove(edge[1]);

if (roots.size() == 0)
throwCyclesInDagException(vertices);

// "Move" the roots detected during this iteration
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
// from the remainingVertices to the DAG
remainingVertices.removeAll(roots);
dag.addAll(roots);

Set<QuickTriggerable> newRemainingVertices = new HashSet<>();
for (QuickTriggerable vertex : vertices)
if (!dag.contains(vertex))
newRemainingVertices.add(vertex);
remainingVertices = newRemainingVertices;

// Compute the new set of remaining edges to continue the iteration
Set<QuickTriggerable[]> newRemainingEdges = new HashSet<>();
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
for (QuickTriggerable[] edge : remainingEdges)
if (!roots.contains(edge[0]))
Expand Down