Free cookie consent management tool by TermsFeed Policy Generator

Opened 8 years ago

Closed 8 years ago

#2536 closed enhancement (done)

Dataset should implement IStringConvertibleMatrix explicitly

Reported by: mkommend Owned by: mkommend
Priority: medium Milestone: HeuristicLab 3.3.14
Component: Problems.DataAnalysis Version: 3.3.13
Keywords: Cc:

Description

The reason is that IStringConvertibleMatrix is only implemented for displaying the values and to provide a default view. The method of IStringConvertibleMatrix should never be used for other reasons and an explicit interface implementation is beneficial to hide those methods.

Change History (9)

comment:1 Changed 8 years ago by mkommend

  • Status changed from new to accepted

comment:2 Changed 8 years ago by mkommend

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

r13427: Implemented IStringConvertibleMatrix explicitly in Dataset.

The only two properties that are not explicitly implemented are Rows and Columns. IMHO these should be implemented explicitly as well and standard properties providing only a getter added. Furthermore, I don't think that counting the rows manually in an integer field gives an benefit compared to

  variableValues.First().Count;
Version 0, edited 8 years ago by mkommend (next)

comment:3 Changed 8 years ago by mkommend

  • Owner changed from bburlacu to gkronber

comment:4 follow-up: Changed 8 years ago by gkronber

  • Owner changed from gkronber to mkommend

Reviewed r13427 (changes are ok).

Do you plan to make the changes proposed in comment 2?

comment:5 Changed 8 years ago by mkommend

r13539: Implemented rows and columns explicitly in the Dataset.

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

Replying to gkronber:

Reviewed r13427 (changes are ok).

Do you plan to make the changes proposed in comment 2?

r13539 implemented the first change proposed in comment 2. I still have to review the implications of changing the rows property.

comment:7 follow-up: Changed 8 years ago by mkommend

  • Owner changed from mkommend to gkronber

I decided to not change the implementation of the row property and leave it as it is => implementation of this ticket is finished.

comment:8 in reply to: ↑ 7 Changed 8 years ago by gkronber

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

Replying to mkommend:

I decided to not change the implementation of the row property and leave it as it is => implementation of this ticket is finished.

Fine by me. Reviewed r13539, please merge to stable.

comment:9 Changed 8 years ago by mkommend

  • Resolution set to done
  • Status changed from readytorelease to closed

r13881: Merged r13427 and r13539 into stable.

Note: See TracTickets for help on using tickets.