Page MenuHomePhabricator

Optimize QmitkChartWidget interface
Open, NormalPublic

Description

Several points to optimize the interface:

  1. AddData* method changes lable automatically

This is problematic because the lable serves also as an identifier. By this implicit change the caller actually looses controll, because there is no feedback if the label was changed or not.
I think the label should be handled as a key in the map. So either if the same lable is used multiple times, one should overwrite (like update) or throw an excpetion because you are not allowed to overwrite an existing lable.

So I would propose the following:

void AddData[1D|2D]( ..., const std::string &label, ChartType chartType = ChartType::bar, bool allowUpdate = true);

If allowUpdate == true, the data will just overwrite existing lables (see UpdateLable). If it is false, it throws an exception if an existing label is reused.

  1. The documentation of AddData is not correct
  1. We should have a method to check if a lable is already used
bool HasExistingLabel (const std::string &label) const;

Event Timeline

floca triaged this task as Normal priority.Jul 9 2019, 4:10 PM
floca created this task.

I can see your point.

There is also

UpdateData[1D|2D](const std::vector<double> &data, const std::string &label)

By using allowUpdate within AddData, we would not need these two update mehods anymore. This would break code where the methods are in use.
I would propose to throw an error if AddData was called with an existing label and throw an error if UpdateData was called with a non-existing label. Both should not happen.

I think it is a better approach to have add and update separated as they are two independent actions.

@floca Do you agree?

Or you want to use the same method AddData multiple times and don't want to switch to UpdateData?

floca added a comment.Jul 22 2019, 4:02 PM

@steint I think I would have made it slightly different, but this is more a case of tast than hard facts (at least I have no good arguments against it right now ;).
So bottom line: We should go with the changes. Thanks. If we learn that something is missing, nothing stops us to revisit and refactor.