GroovyFX
  1. GroovyFX
  2. GFX-16

avoid lengthy switches on type code inside builder factories

    Details

    • Type: Improvement Improvement
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      when looking through some GroovyFX code with @aalmiray,
      he frowned at lengthy switches on type codes as e.g. in TransformFactory.

      The alternative would be to use separate builder instances as e.g. with ShapeFactory.

        Activity

        Hide
        Dean Iverson added a comment - - edited

        When Andres first voiced his disapproval of the switch statements, I converted a couple of the worst offenders and profiled. It basically comes down to a slight gain in performance (separate instances) vs a slight savings in memory overhead (one instance with switch). The differences were minimal, so I didn't bother converting the rest of the factories.

        Given that, it's mostly a code style issue. If you feel strongly enough about it that you want to refactor the rest of the factories, I say go for it!

        Show
        Dean Iverson added a comment - - edited When Andres first voiced his disapproval of the switch statements, I converted a couple of the worst offenders and profiled. It basically comes down to a slight gain in performance (separate instances) vs a slight savings in memory overhead (one instance with switch). The differences were minimal, so I didn't bother converting the rest of the factories. Given that, it's mostly a code style issue. If you feel strongly enough about it that you want to refactor the rest of the factories, I say go for it!
        Hide
        Andres Almiray added a comment -

        Though performance wise there doesn't seem to be so much gain, when reading the code the history is different. Also, providing a hierarchy of factories can help in registering 3rd part components (like those created by the awesome @hansolo_). So, limiting ShapeFactory (for example) to those nodes provided only by javaFX SDK is not a good idea Same principle applies to all other switching factories.

        One last point. If an agreement is reached regarding builder metadata it's very likely that the groovyfx factories will need to be refactored to follow the same model as the rest of FBS based builders.

        My $0.02

        Show
        Andres Almiray added a comment - Though performance wise there doesn't seem to be so much gain, when reading the code the history is different. Also, providing a hierarchy of factories can help in registering 3rd part components (like those created by the awesome @hansolo_). So, limiting ShapeFactory (for example) to those nodes provided only by javaFX SDK is not a good idea Same principle applies to all other switching factories. One last point. If an agreement is reached regarding builder metadata it's very likely that the groovyfx factories will need to be refactored to follow the same model as the rest of FBS based builders. My $0.02
        Hide
        Dean Iverson added a comment - - edited

        The voice of experience speaks.

        Those are all good points. Perhaps this refactoring should be included in the 0.1 release target?

        Show
        Dean Iverson added a comment - - edited The voice of experience speaks. Those are all good points. Perhaps this refactoring should be included in the 0.1 release target?

          People

          • Assignee:
            Dean Iverson
            Reporter:
            Dierk König
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: