Opened 8 years ago

Closed 8 years ago

#1269 closed enhancement (done)

Add RowsChanged, ColumnsChanged event to ValueTypeMatrix<T>

Reported by: abeham Owned by: mkommend
Priority: medium Milestone: HeuristicLab 3.3.3
Component: Data Version: 3.3.3
Keywords: Cc:

Description

Parameter binding branch:

Currently the only event that identifies a change in the matrix dimensions is Reset. Additionally RowsChanged and ColumnsChanged should be fired so that an automatic binding mechanism can be applied that takes the number of rows into account.

Change History (15)

comment:1 Changed 8 years ago by swagner

  • Owner changed from abeham to mkommend
  • Priority changed from high to medium
  • Status changed from new to assigned
  • Version changed from branch to 3.3.2

comment:2 Changed 8 years ago by swagner

  • Milestone changed from HeuristicLab x.x.x to HeuristicLab 3.3.3

comment:3 Changed 8 years ago by mkommend

  • Status changed from assigned to accepted

comment:4 Changed 8 years ago by mkommend

Added ColumnsChanged and RowsChanged event in IStringConvertibleMatrix and all implementing classes with r5150.

comment:5 Changed 8 years ago by mkommend

  • Owner changed from mkommend to abeham
  • Status changed from accepted to reviewing

Please forward the ticket to swagner after your review is finished.

comment:6 follow-up: Changed 8 years ago by abeham

  • Owner changed from abeham to mkommend
  • Status changed from reviewing to assigned

RunCollection

  • ItemRemoved should fire RowsChanged as well.
  • From the point of view of an IStringConvertibleMatrix AddParameter, AddResult, RemoveParameterName, and RemoveResultName may also result in a change to the value returned by the Columns property.

comment:7 Changed 8 years ago by mkommend

  • Status changed from assigned to accepted

comment:8 Changed 8 years ago by mkommend

Added OnRowsChanged in RunCollection.OnItemsRemoved with r5152.

comment:9 in reply to: ↑ 6 Changed 8 years ago by mkommend

  • Owner changed from mkommend to abeham
  • Status changed from accepted to reviewing

From the point of view of an IStringConvertibleMatrix AddParameter, AddResult, RemoveParameterName, and RemoveResultName may also result in a change to the value returned by the Columns property.

The listed methods are only called from OnItemsAdded, OnItemsRemoved, etc. which handle the firing of events to improve the performance of the RunCollection.

comment:10 follow-up: Changed 8 years ago by abeham

  • Owner changed from abeham to swagner
  • Status changed from reviewing to assigned

Okay. As per your request, I'll forward this to swagner.

swagner please have a look at the events that are fired. As discussed with mkommend, there are *NameChanged and *Changed events that are fired right after another. Unfortunately I don't have a general recipe myself in situations where multiple events ought to be fired. From my point of view, I think we should hide e.g. the row name change, when the row changes itself, so that a RowsChanged event dominates a RowNamesChanged event. It should be pretty clear that when the rows have changed, the names need to have changed as well.

comment:11 Changed 8 years ago by swagner

  • Status changed from assigned to reviewing

comment:12 in reply to: ↑ 10 Changed 8 years ago by swagner

Replying to abeham:

swagner please have a look at the events that are fired. As discussed with mkommend, there are *NameChanged and *Changed events that are fired right after another. Unfortunately I don't have a general recipe myself in situations where multiple events ought to be fired. From my point of view, I think we should hide e.g. the row name change, when the row changes itself, so that a RowsChanged event dominates a RowNamesChanged event. It should be pretty clear that when the rows have changed, the names need to have changed as well.

I am not so happy with the idea to suppress events, as then it might become quite cumbersome to understand which events are fired in which cases. Furthermore, consider the case when someone is just interested in name changes. If we suppress some of the name changed events, handlers for both events have to be registered which might not be intuitively clear. Therefore I would recommend to fire multiple events, as long as we do not run into any performance problems.

comment:13 Changed 8 years ago by swagner

I reviewed r5150 and r5152 and do not have any additional comments. Thanks.

comment:14 Changed 8 years ago by swagner

  • Owner changed from swagner to mkommend
  • Status changed from reviewing to readytorelease

comment:15 Changed 8 years ago by mkommend

  • Resolution set to done
  • Status changed from readytorelease to closed
  • Version changed from 3.3.2 to 3.3.3
Note: See TracTickets for help on using tickets.