Opened 5 years ago

Closed 5 years ago

#14960 closed defect (fixed)

Remove asserts on wxGrid HideCol/HideRow/ShowCol/ShowRow

Reported by: hackish Owned by:
Priority: normal Milestone:
Component: wxGrid Version: stable-latest
Keywords: Cc:
Blocked By: Blocking:
Patch: no


As per the discussion on the wx-dev list:
MR> I've been using the wxGrid components pretty extensively in an application
MR> I'm writing. Unfortunately the latest round of changes on the wxGrid
MR> requires that the caller know if a row/column is hidden before telling the
MR> grid to hide it rather than ignoring the call or re-doing the work to hide
MR> an already hidden column.
MR> While it may technically be incorrect to call hide on an already hidden
MR> item, the grid itself is the best equipped item to determine the actions
MR> needed. Making the call does not leave the grid in an error state. At
MR> present the programmer is forced to store visibility info or query the grid
MR> as to the visibility, then make or not make the call. If the call is made
MR> the method re-checks the visibility and proceeds. At present you have at
MR> minimum double the amount of space or time complexity.

I agree with your argument. I didn't think the time complexity would be
important in this case but maybe I was wrong for grids with that many
rows/columns. I still disagree about the space complexity, IMHO it's worth
it because of the extra clarity it provides in the code but it's true that
there is currently no efficient way to ensure that a column is hidden
independently of whether it's currently hidden or shown. And as it seems
not worth it to add another UnconditionallyHideCol() method to wxGrid, I
agree that we should remove the asserts.

AFAICS doing it should be quite simple, we just need to return 0 from
UpdateRowOrColSize() in src/generic/grid.cpp if the assert condition is
false. But I don't have test code for this so it's not that simple for me
to check that my changes work correctly. If you could please make them,
verify that they work with your program and put a patch with them on Trac
it would be very much appreciated.

Thanks in advance,

Attachments (1)

gridassert.patch download (3.4 KB) - added by hackish 5 years ago.
Patch corrects the problem and adds to the sample to allow the feature to be exercised.

Download all attachments as: .zip

Change History (3)

Changed 5 years ago by hackish

Patch corrects the problem and adds to the sample to allow the feature to be exercised.

comment:1 Changed 5 years ago by hackish

  • Summary changed from Remove assets on wxGrid HideCol/HideRow/ShowCol/ShowRow to Remove asserts on wxGrid HideCol/HideRow/ShowCol/ShowRow

comment:2 Changed 5 years ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [73366]) Allow hiding/showing already hidden/shown wxGrid rows/columns.

Don't assert if an already hidden/shown row/column is being hidden/shown again
but simply don't do anything. This is more convenient because the code outside
wxGrid has no efficient way to only hide a row/column if it's currently shown.

Closes #14960.

Note: See TracTickets for help on using tickets.