Re: [lmi] Correctness and performance of varying column width mode in the census view

[Vadim Zeitlin]

On Wed, 27 May 2020 16:56:46 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2020-05-27 16:22, Vadim Zeitlin wrote:
GC> > 
GC> >  I'm struggling with choosing the right two out of the following three
GC> > goals for the automatic column fitting in the census view:
GC> > 
GC> > 1. Correct.
GC> > 2. Fast.
GC> > 3. Simple.
GC> > 
GC> > and would like to ask for your help with determining the ones to pursue.
GC> 
GC> If we define correctness as
GC>  - a valid input file always produces valid reports, and
GC>  - invalid input never produces invalid reports
GC> then that type of correctness is paramount. But I think you're using
GC> a different definition here.

 Yes, absolutely, I'm using "correct" in a very narrow sense and all it
means here is that the columns always have the size exactly fitting their
contents when "varying column width" is enabled.

GC> Suppose we have a field containing a person's name, say, "John Brown",
GC> and (pretending this is monospace ASCII) those ten letters are rendered
GC> in a column of width eleven (pretending that the column separator is a
GC> single ASCII space). Now the end user changes that one field, in that
GC> one column, to "Sojourner Truth". If we don't change the column width,
GC> the screen shows "Sojourner" because that's all that fits, " Truth"
GC> being truncated; is that truncation your operative definition of
GC> incorrectness?

 Yes.

GC> I'm hoping that's all it is, because I think that truncation is not
GC> really unreasonable, as it's what spreadsheet programs do. AIUI,
GC> for spreadsheets, "resize column widths" is a verb--whereas lmi has
GC> (at least until now) offered such a behavior as a sticky mode, in
GC> which the verb is called whenever any contents change (in that mode).

 Yes, exactly. And we have CensusView::autosize_columns_ which stores the
current mode.

GC> While that "always resize everything whenever anything changes" mode
GC> does have a certain charm for a tiny census, it seems far too costly
GC> for a large number of columns and rows.

 Currently it is indeed far too costly, but there is no intrinsic reason
for it to be so, at least not when changing a single cell, because all we
need to do is to measure the length of a single string and increase the
column width if the new string doesn't fit into it.

 Now there are other complications, e.g. the operation would still be O(N),
where N is the number of rows, if we wanted to also reduce the column
width, but IMO this is much less important and we could avoid doing this.
There are also operations affecting all cells, such as "Edit class/case",
but those are presumably less frequent than editing individual cells, so
perhaps it's not as annoying that they take so long.

GC> AFAICS lmi's GUI presents it as a verb. IOW, there are separate toolbar
GC> buttons for "fixed" and "variable", rather than a single button that's
GC> either depressed or not. Thus, the stickiness of the notional mode is
GC> just an internal artifact that we can and should eradicate.

 This is a huge surprise to me because I was absolutely convinced that this
was a conscious decision and took it for granted that we wanted to keep it.
I don't know why exactly did I believe this, I'd like to think there was
some reason for it, but it's probably not really important to spend time on
finding the exact reason in the list archives or the commit messages if
you're sure that we don't want to do this anyhow... (and perhaps I'm
misremembering things again and there never was any such reason).

GC> Does that answer all your questions?

 It almost does. But, to be sure that I understood correctly, let me
confirm which UI operations precisely will still need to auto-size the
columns:

- Toolbar/menu command "Varying column widths" will do it, of course.

- Editing/adding/deleting cells will not do it, according to your answer,
  so there is no need to optimize doing it.

- But what about the operations affecting all cells (and so already
  taking O(N) time), such as editing class/case or pasting census
  from clipboard, should they still resize the columns to fit their
  contents?

 Personally I think these operations should still honour auto-size mode if
it's enabled, both to avoid changing the current behaviour without a good
reason, and to keep the changes in the code to the minimum, because if we
don't do this neither, we would need to get rid of the previously mentioned
CensusView::autosize_columns_ when using wxGrid too (while keeping it for
wxDVC-based version, whose behaviour shouldn't change). But then I also
think we should do it for editing cells as well, just in a smart -- and
fast -- way, so I wouldn't be really surprised if you think differently.

 Please let me know if you do,
VZ

pgpQ45HLKq8E1.pgp
Description: PGP signature