1. McGill CORE Avatar McGill CORE
  2. TouchCORE
  3. touchram
  4. Issues

Aggregation fill color independent of background color

Issue #86 resolved
Matthias Schoettle created an issue

The polygon used for the aggregation for associations (it's the CompositionPolygon from RelationshipView) has a fixed filling. If it is shown somewhere else (for example, in the split view), it has the wrong fill colour. However, not filling it doesn't work, because the line of the relationship goes right through it.

Either adjust the fill colour to proper the background colour or draw the line from the end of the polygon (and not the end of the component).

Comments (12)

  3. Thomas Di Meco

    I solved this issue by adding a background color (or "secondary color") for RelationshipView which the StructuralDiagramView can modify depending on their own background color. I also moved the structural diagram background color information to the StructuralDiagramView to be homogeneous (because in a split view, the bottom color is held by the structural diagram view and the top color by the scene view). I checked, it normally should not be a problem, but I am not 100% sure.

  4. Matthias Schoettle reporter

    Looks good, thank you! I think previously there was a reason for the StructuralDiagramView to have no background color, but I cannot recall why. And it works.

    The only thing that would be nice is if you could fix the Checkstyle issues of your changes, please. :) Then it can be merged into master.

  6. Thomas Di Meco

    Ok thank you! Feel free to tell me in the future if I broke something with the StructuralDiagramView background color.

    A last (little) question, in the RelationshipView class, line 260, Checkstyle tells me to make my field private. Should I do it? Is it more clean?

  7. Matthias Schoettle reporter

    Thanks!

    Oh I didn't realize that. The check is to enforce/ensure encapsulation (information hiding), but I think in the case of protected it should be okay to do it. Otherwise we need getter/setter for each of them, which will make the classes more cluttered. Unless the property is never needed in a subclass, then I would make it private.

    Maybe we should relax this check, what do you think?

  8. Thomas Di Meco

    Mmm it's a great debate :-) Indeed, strict encapsulation don't recommend using protected fields because the field is managed by the base class. But in case of high related classes, this can avoid some "useless" code... It depends on how much you want the code to be strict and, well... clean maybe ^^

  9. Matthias Schoettle reporter

    Yes, it is a big debate :) I think it could be a good compromise between strict and clean code to allow protected fields and the current rules already enforce the developer to provide a Javadoc comment for public and protected fields.

    What do you think? (feel free to post your opinion to the mailing list)

  10. Matthias Schoettle reporter

    Would you be able to address the Checkstyle for the protected field (configuration is updated) and merge it into master, please? Thanks :)

  13. Log in to comment
Assignee
Type
bug
Priority
major
Status
resolved
Component
–
Milestone
AOSD 2014 Demo
Votes
0
Watchers
2
Jira: the preferred issue tracker for Bitbucket. Join the team!