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.

Sign up for GitHub

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

Jump to bottom

Improve design of edge constraints checking #416

Closed
prmr opened this issue Feb 22, 2021 · 0 comments
Closed

Improve design of edge constraints checking #416

prmr opened this issue Feb 22, 2021 · 0 comments
Labels
main Issues with the main code base refactoring A proposal to refector or generally improve the code
Milestone
Release 3.2

Comments

@prmr
Copy link
Owner

prmr commented Feb 22, 2021

The edge validation code requires the construction of an entirely new ConstraintSet object for every call, which is not efficient. We can refactor the code so that we only need one ConstraintSet per diagram type.
@prmr prmr added refactoring A proposal to refector or generally improve the code main Issues with the main code base labels Feb 22, 2021
@prmr prmr added this to the Release 3.2 milestone Feb 22, 2021
prmr added a commit that referenced this issue Feb 22, 2021
@prmr
#416 Move creation of ConstructorCall to SequenceDiagramBuilder
9606441 
When a user attempts to create a constructor call, they use the normal
"CallEdge" creation tool from the tool bar. This design decision was to
avoid forcing the user to use two very similar tools to create call and
constructor edges. However, internally if a constructor call is created,
a ConstructorEdge must be used, so it's necessary to convert the
original CallEdge to a Constructor Edge. This was initially done in the
DiagramCanvasController, leading to a special case and coupling with the
SequenceDiagram Builder. To avoid this design weakness, the conversion
from CallEdge to ConstructorEdge is moved to the SequenceDiagramBuilder.
Farihatanjin pushed a commit to Farihatanjin/JetUML that referenced this issue Feb 23, 2021
@prmr @Farihatanjin
prmr#416 Move creation of ConstructorCall to SequenceDiagramBuilder
93dd64b 
When a user attempts to create a constructor call, they use the normal
"CallEdge" creation tool from the tool bar. This design decision was to
avoid forcing the user to use two very similar tools to create call and
constructor edges. However, internally if a constructor call is created,
a ConstructorEdge must be used, so it's necessary to convert the
original CallEdge to a Constructor Edge. This was initially done in the
DiagramCanvasController, leading to a special case and coupling with the
SequenceDiagram Builder. To avoid this design weakness, the conversion
from CallEdge to ConstructorEdge is moved to the SequenceDiagramBuilder.
prmr added a commit that referenced this issue Feb 23, 2021
@prmr
#416 Refactor constraint about edge end in sequence diagram
adb9e2a 
The case where trying to create a constructor edge was implemented as a
special case in constraints, where the constraint was only added to the
constraint set if the call could be a legal constructor call. This
divergence from the design, where all constraints are stored in a set,
was creating an obstacle to the future improvement of the constraint
management. With this change, all constraints are implemented as
predicates.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Feb 24, 2021
@Farihatanjin
prmr#416 Refactor edge constraints checking
34694f9 
`ConstraintSet` is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new `ConstraintSet` objects each time `canAdd` is
called. For this reason, I removed the `merge` method in `ConstraintSet`
which is no longer necessary.
The `ConstraintSet` field can be accessed by method
`getEdgeConstraints()` which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a `ConstraintSet` which will be
returned.
 `Satisfied` takes in the `canAdd` parameters in order to check the
conditions whilst keeping the code simple.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Feb 24, 2021
@Farihatanjin
prmr#416 Refactor edge constraints checking (bug fixes)
7a97476 
ConstraintSet is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new ConstraintSet objects each time canAdd is
called. For this reason, I removed the `merge` method in ConstraintSet
which is no longer necessary.
The ConstraintSet field can be accessed by method
getEdgeConstraints() which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a ConstraintSet which will be
returned.
Satisfied takes in the canAdd parameters in order to check the
conditions whilst keeping the code simple.
yannsartori added a commit to yannsartori/JetUML that referenced this issue Feb 24, 2021
@yannsartori
prmr#416 Fix failing tests for Mac
f4ec885
prmr pushed a commit that referenced this issue Feb 25, 2021
@yannsartori @prmr
#416 Fix failing tests for Mac
a34605c
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Feb 25, 2021
@Farihatanjin
prmr#416 Refactor edge constraints checking
9a568a6 
`ConstraintSet` is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new `ConstraintSet` objects each time `canAdd` is
called. For this reason, I removed the `merge` method in `ConstraintSet`
which is no longer necessary.
The `ConstraintSet` field can be accessed by method
`getEdgeConstraints()` which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a `ConstraintSet` which will be
returned.
 `Satisfied` takes in the `canAdd` parameters in order to check the
conditions whilst keeping the code simple.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Feb 25, 2021
@Farihatanjin
prmr#416 Refactor edge constraints checking (bug fixes)
662f41e 
ConstraintSet is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new ConstraintSet objects each time canAdd is
called. For this reason, I removed the `merge` method in ConstraintSet
which is no longer necessary.
The ConstraintSet field can be accessed by method
getEdgeConstraints() which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a ConstraintSet which will be
returned.
Satisfied takes in the canAdd parameters in order to check the
conditions whilst keeping the code simple.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Feb 25, 2021
@Farihatanjin
prmr#416 rebase to fix broken tests
7b0ecdd
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
prmr#416 rebase
619eaff
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
prmr#416 rebase
724e450
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
Revert "prmr#416 rebase"
e94c3bf 
This reverts commit 724e450.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
Revert "prmr#416 rebase"
74af578 
This reverts commit 619eaff.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
prmr#416 Refactor edge constraints checking
b8a4b99 
`ConstraintSet` is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new `ConstraintSet` objects each time `canAdd` is
called. For this reason, I removed the `merge` method in `ConstraintSet`
which is no longer necessary.
The `ConstraintSet` field can be accessed by method
`getEdgeConstraints()` which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a `ConstraintSet` which will be
returned.
 `Satisfied` takes in the `canAdd` parameters in order to check the
conditions whilst keeping the code simple.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
prmr#416 Refactor edge constraints checking (bug fixes)
daa5e93 
ConstraintSet is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new ConstraintSet objects each time canAdd is
called. For this reason, I removed the `merge` method in ConstraintSet
which is no longer necessary.
The ConstraintSet field can be accessed by method
getEdgeConstraints() which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a ConstraintSet which will be
returned.
Satisfied takes in the canAdd parameters in order to check the
conditions whilst keeping the code simple.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
prmr#416 rebase to fix broken tests
02d7565
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 6, 2021
@Farihatanjin
Revert "prmr#416 rebase"
7353498 
This reverts commit 724e450.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 8, 2021
@Farihatanjin
prmr#416 refactor edge constraints checking
3aefb24 
ConstraintSet is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new ConstraintSet objects each time canAdd is
called. For this reason, I removed the `merge` method in ConstraintSet
which is no longer necessary.
The ConstraintSet field can be accessed by method
getEdgeConstraints() which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a ConstraintSet which will be
returned.
Satisfied takes in the canAdd parameters in order to check the
conditions while keeping the code simple.
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 8, 2021
@Farihatanjin
prmr#416 fix issues with PR
35d40d0
Farihatanjin added a commit to Farihatanjin/JetUML that referenced this issue Mar 8, 2021
@Farihatanjin
prmr#416 delete classpath
ce9f5b5
yingjie-xu pushed a commit to yingjie-xu/JetUML that referenced this issue Mar 9, 2021
@prmr @yingjie-xu
prmr#416 Move creation of ConstructorCall to SequenceDiagramBuilder
fdd4b0f 
When a user attempts to create a constructor call, they use the normal
"CallEdge" creation tool from the tool bar. This design decision was to
avoid forcing the user to use two very similar tools to create call and
constructor edges. However, internally if a constructor call is created,
a ConstructorEdge must be used, so it's necessary to convert the
original CallEdge to a Constructor Edge. This was initially done in the
DiagramCanvasController, leading to a special case and coupling with the
SequenceDiagram Builder. To avoid this design weakness, the conversion
from CallEdge to ConstructorEdge is moved to the SequenceDiagramBuilder.
yingjie-xu pushed a commit to yingjie-xu/JetUML that referenced this issue Mar 9, 2021
@prmr @yingjie-xu
prmr#416 Refactor constraint about edge end in sequence diagram
f31bd42 
The case where trying to create a constructor edge was implemented as a
special case in constraints, where the constraint was only added to the
constraint set if the call could be a legal constructor call. This
divergence from the design, where all constraints are stored in a set,
was creating an obstacle to the future improvement of the constraint
management. With this change, all constraints are implemented as
predicates.
yingjie-xu pushed a commit to yingjie-xu/JetUML that referenced this issue Mar 9, 2021
@yannsartori @yingjie-xu
prmr#416 Fix failing tests for Mac
9bbf0e4
prmr pushed a commit that referenced this issue Mar 9, 2021
@Farihatanjin @prmr
#416 refactor edge constraints checking
4009e94 
ConstraintSet is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new ConstraintSet objects each time canAdd is
called. For this reason, I removed the `merge` method in ConstraintSet
which is no longer necessary.
The ConstraintSet field can be accessed by method
getEdgeConstraints() which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a ConstraintSet which will be
returned.
Satisfied takes in the canAdd parameters in order to check the
conditions while keeping the code simple.
prmr pushed a commit that referenced this issue Mar 9, 2021
@Farihatanjin @prmr
#416 fix issues with PR
703e57a
prmr pushed a commit that referenced this issue Mar 9, 2021
@Farihatanjin @prmr
#416 delete classpath
a912212
prmr added a commit that referenced this issue Mar 9, 2021
@prmr
#416 Restore classpath
6b0811a
prmr added a commit that referenced this issue Mar 9, 2021
@prmr
#416 Fix the documentation
0786851
@prmr prmr closed this as completed Mar 9, 2021
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@prmr @louib
prmr#416 Move creation of ConstructorCall to SequenceDiagramBuilder
cb1bb19 
When a user attempts to create a constructor call, they use the normal
"CallEdge" creation tool from the tool bar. This design decision was to
avoid forcing the user to use two very similar tools to create call and
constructor edges. However, internally if a constructor call is created,
a ConstructorEdge must be used, so it's necessary to convert the
original CallEdge to a Constructor Edge. This was initially done in the
DiagramCanvasController, leading to a special case and coupling with the
SequenceDiagram Builder. To avoid this design weakness, the conversion
from CallEdge to ConstructorEdge is moved to the SequenceDiagramBuilder.
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@prmr @louib
prmr#416 Refactor constraint about edge end in sequence diagram
254728c 
The case where trying to create a constructor edge was implemented as a
special case in constraints, where the constraint was only added to the
constraint set if the call could be a legal constructor call. This
divergence from the design, where all constraints are stored in a set,
was creating an obstacle to the future improvement of the constraint
management. With this change, all constraints are implemented as
predicates.
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@yannsartori @louib
prmr#416 Fix failing tests for Mac
6973c54
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@Farihatanjin @louib
prmr#416 refactor edge constraints checking
20fd32b 
ConstraintSet is now stored as a private static final field in each
diagram builder. This reduces object creation by eliminating the need to
create and merge new ConstraintSet objects each time canAdd is
called. For this reason, I removed the `merge` method in ConstraintSet
which is no longer necessary.
The ConstraintSet field can be accessed by method
getEdgeConstraints() which simply returns the set of constraints. I
see no encapsulation issue in doing so since there is no method
available to modify contents of a ConstraintSet which will be
returned.
Satisfied takes in the canAdd parameters in order to check the
conditions while keeping the code simple.
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@Farihatanjin @louib
prmr#416 fix issues with PR
3aee356
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@Farihatanjin @louib
prmr#416 delete classpath
07f2112
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@prmr @louib
prmr#416 Restore classpath
0551493
louib pushed a commit to louib/JetUML that referenced this issue Aug 15, 2021
@prmr @louib
prmr#416 Fix the documentation
689b4b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
main Issues with the main code base refactoring A proposal to refector or generally improve the code
Projects
None yet
Milestone
Release 3.2
Development

No branches or pull requests
1 participant
@prmr