Code review for quality control
Objectives of code review:
- Provide good foundation for future development
- Anticipate plugin development
Blueprint information
- Status:
- Complete
- Approver:
- None
- Priority:
- High
- Drafter:
- None
- Direction:
- Approved
- Assignee:
- None
- Definition:
- Obsolete
- Series goal:
- None
- Implementation:
- Unknown
- Milestone target:
- None
- Started by
- Completed by
- Aiman Baharna
Related branches
Related bugs
Sprints
Whiteboard
Issues that are identified:
(1) Do not store iters in Row objects
- Row.Iter used by ActionMoveRows.
- Row.Iter used by ActionDeleteRow
- Row.Iter used by ActionDeleteRow
- Row.Iter used by ActionDeleteRow
- Row.Iter used by Document.
(2) The use of ActionUndo seems wrong
- There is no need for this overcomplication.
(3) The use of Document.
(4) The way changes to cells and corresponding event notifications seems wrong
- Currently all access to cells happens via Row.GetCell
- Currently all changes to cells *must* happen via Row.SetCell
- Then Row.SetCell calls Document.
- In RecordActions mode, Document.
- Ideal solution:
- It seems more intuitive to let Document handle cell changes not the row
- There should not be any need to call SetValue or even SetCell
- Cell properties should automatically notify the document
- Problem: how to tell whether change should warrant an undo action?
- Answer:
- Create explicit Document.SetValue with "record" boolean arg
- When change is notified from a cell, record=False by default
(5) It seems weird to decouple view columns from data columns
- The reason for separation needs to be documented some place
(6) UNDECIDED: Whether to have a base column and how to deal with it?
- Have I decided how the base column is going to be treated?
- Is an explicit base column necessary, or is it just going to be a free-for-all
- How to deal with optional settings on the base column?
(7) The program is not plugin-ready:
- The way things are coded uses too many "type field" instead of virtual functions
- Need to create some kind of global hashtables for cell types, column types, view columns, etc.
- Plugins can hook into these by adding new entries to these hash tables, or overriding values, etc
(8) Column traversal on update of a cell needs revamp
- Need to implement some generic reusable traversal functions, currently they are all local
- Need to make the checkbox summary traversal more efficient instead of traversing the whole tree
- Need to prevent infinite recursion when updating cell value triggers another column update
- Need to prevent undo actions being triggered when updating cell value during column update
- Currently infinite recursion is prevented with a variable column.Updating
(9) Fix obsolete Gtk method calls which are generating compiler warnings.
(10) Investigate any potential memory leaks relating to TreeIter or TreePath usage
- May need to defer this goal because it is a big undertaking