wiki:UNPLANNED_LOGIC_REFACTORING_R0
Last modified 16 years ago Last modified on 09/11/09 10:52:53

Error: Macro BackLinksMenu(None) failed
compressed data is corrupt

Error: Macro TicketQuery(summary=UNPLANNED_LOGIC_REFACTORING_R0, format=table, col=summary|owner|status|type|component|priority|effort|importance, rows=description|analysis_owners|analysis_reviewers|analysis_score|design_owners|design_reviewers|design_score|implementation_owners|implementation_reviewers|implementation_score|test_owners|test_reviewers|test_score|) failed
current transaction is aborted, commands ignored until end of transaction block

Analysis

Overview

This task is about refactoring the logic operations in the application. The problems related to that include (but are not limited to):

  • There are operations that do not use R3 logic.
  • There is code duplication (especially in the menu items and bound controls in HUDs).
  • There is no convention on naming the events and handlers.
  • There is no convention on placing (and grouping) the logic classes.

After this iteration there should be a convention for placing, grouping and naming events and handlers. All existing R3 logics should follow that convention. A lot of the code smells should be eliminated. All old logics will be replaced by new ones. This task is with a bigger scope and might not be finished in one week.

Task requirements

  • Create a convention for naming, placing and grouping EventIds, OperationDefs and entries in OperationDefs, apply it and start following it.
  • Identify and remove duplicate code.
  • Refactor old logic classes to use LogicR3.
    • Use LogicR3 handling instead of direct method invocation.
    • All buttons, menu items and bound controls should fire an event.
    • Use AutoActions when you want to change the model. They should perform a single ModelChange and have a good description.

Task result

  • Convention for Logics, described in the design section of this page.
  • Source code

Implementation idea

  • It seems sensible that all nested ids for events should be named EventIds. Since they are accessed by the wrapper class, it is clear what event they are for. E.g. MenuItem.EventIds.CLICKED.
  • OperationDef members that handle an event might be named starting with ON_ (e.g. ON_SET_SCREEN_MODE).
  • In the MenuItem there is code duplication on the clicked method. It should become final and fire a CLICKED event.

UNPLANNED_MAIN_WINDOW_REFACTORING_R0
GROUP_BOOK_MODEL_REDESIGN_R0

How to demo

  • Show the convention to everyone.
  • Show the code and demonstrate that there are no old logics left.
  • Run the available tests.

Design

Since this is a redesign task with critical priority, the design will be described briefly:

Naming convention

  • All enum classes that implement OperationDef should end in Logic.
    • Examples: PlainTextLogic, ResourcePaletteLogic, etc.
    • The name should be self-descriptive about what kind of operations this logic contains.
  • Each operation should start with ON_
    • Then follows the user action that this operation handles. This should also be self-descriptive.
    • Examples: ON_SET_SCREEN_MODE, ON_ADD_AUDIO_FRAME, etc.
  • The EventIds enum should be an inner class of the class that fires the event when possible and should all be with the same name.
    • The event ids themselves should clearly describe the operation they represent.
    • It should be clear from how the event id is referred (via the wrapper class name) what event it represents.
    • Examples: MainWindow.EventIds.SET_SCREEN_MODE, AllAnnotationsPalette.EventIds.IMPORT_ANNOTATIONS, etc.
  • There might be some cases when these rules cannot be applied (e.g. TextEventIds) but this is the convention that we will try to follow.

Duplicate code

The following places have been identified to contain unnecessary code:

  • The clicked() method of the menu items. It should be made final, firing an event with a MenuItem.EventIds.CLICKED id. Subclasses do not need to override this method.
  • The submitData() method of the bound controls. It should be made final, firing an event with a BoundControl.EventIds.SUBMIT id. Subclasses do not need to override this method.
  • The situation is similar with the AlignElementsButton. Here's mitex's solution:
  • For the align buttons:
    • Create two abstract methods in AlignElementsButton:
      • float calculateAlignValue(List<ElementView> selected) - for a given list of element views calculates some align value, for example, the Y-axis of the top-most element.
      • ImmPoint calculateNewLocation(ImmRect bounds, float alignValue) - calculates a new location for a single element, given the old bounds and the align value, returned from the above method.
    • Replace the clicked method in all align buttons with the above methods.
    • Create AlignElementsLogic with a single handler - ON_ALIGN, which listens for CLICKED from AlignElementsButton.
      • Calculates the align value
      • Creates an immutable list of new locations
      • Creates an immutable list of references to elements. Only elements of type MemberElement are collected.
      • Creates a new AutoAction that iterates over all references and changes MemberElement.KEY_LOCATION with the newly calculated location.

Other places with duplicate code will be identified and fixed as part of the implementation.

LogicR3 refactoring

All existing R3 logics will be changed to use auto actions. They will also be grouped by common functionality. Classes and methods using old logic and trying to set properties of the model will be refactored as well. These things do not need preliminary research and will be done as part of the implementation.

Implementation

Changesets:

This task is declared completed, although a lot of things are left to be done. This is due to its huge scope and the difficulty of reviewing a large task.

After this iteration the following has been achieved:

  • A convention for naming logics has been created. It is described in the Design section of this document.
  • Most of the duplicate code identified in the design has been removed.
  • Old logic classes have been deleted and their functionality has been moved to new logics.
  • Unimplemented things have been marked with a "FIXME (R4)" tag.
  • SortKeys have been added to some logics.

The following needs to be done at the next iteration:

  • Nested logics in some huds and halos need to be extracted.
  • Some bound controls still override the submitData() method, this should be fixed.
  • All "FIXME (R4)" logics should be reimplemented with the new design.
  • SortKeys should be added to all logics.

Testing

Place the testing results here.

Comments

Write comments for this or later revisions here.