Review Request 122278: Additional changes in section handling

[Inge Wallin]

On Thursday, January 29, 2015 14:11:39 Inge Wallin wrote:
> On Thursday, January 29, 2015 12:57:35 Denis Kuplyakov wrote:
> > > On Янв. 27, 2015, 7:34 п.п., Camilla Boemann wrote:
> > > > libs/kotext/KoSectionManager.cpp, line 120
> > > > <https://git.reviewboard.kde.org/r/122278/diff/1/?file=345390#file3453
> > > > 90
> > > > line120>> >
> > > > 
> > > >     why no long const ?
> > > 
> > > Denis Kuplyakov wrote:
> > >     I don't know why, but QHash<T,Q>::find() isn't const. Thats why I
> > >     can't make this function const and possibleNewName() too as
> > >     dependant.>
> > > 
> > > Denis Kuplyakov wrote:
> > >     Ohh, I see constFind there. Why they made it such way? Anyway, will
> > >     fix this.>
> > > 
> > > Denis Kuplyakov wrote:
> > >     There is a sectionNames() call, that updates internal data, to
> > >     prevent
> > >     using of invalidated one. So we can't put const here.>
> > > 
> > > Friedrich W. H. Kossebau wrote:
> > >     Could the members which hold that data that is only lazily
> > >     fetched/updated be tagged with `mutable` perhaps? Problem with
> > >     non-const get-methods is that this escalates up in the call
> > >     hierarchy, i.e. any other method which uses this method now also
> > >     needs to be non-const. Which is not nice, because it spoils the
> > >     usefulness of const, which should protect against unintended data
> > >     modification. Which lazily updated data surely is not, that's e.g.
> > >     why there is `mutable`.>
> > > 
> > > Denis Kuplyakov wrote:
> > >     Can you post me some link, where I can read about what you mean
> > >     under
> > >     mutable?>
> > > 
> > > Friedrich W. H. Kossebau wrote:
> > >     Any good physical C++ book, hard to link ;) (no idea about what
> > >     related ebooks there are)
> > >     
> > >     Hm, perhaps this here gives you a start:
> > >     http://en.cppreference.com/w/cpp/language/cv>
> > > 
> > > Elvis Stansvik wrote:
> > >     http://en.cppreference.com/w/cpp/language/cv - "mutable - applies to
> > >     non-static class members of non-reference non-const type and
> > >     specifies that the member does not affect the externally visible
> > >     state of the class. mutable members of const classes are modifiable
> > >     (Note: the C++ language grammar treats mutable as a
> > >     storage-class-specifier, but it does not affect storage class)".
> > >     Wikipedia has a slightly less technical description (second
> > >     example):
> > >     http://en.wikipedia.org/wiki/Immutable_object#C.2B.2B>
> > > 
> > > Denis Kuplyakov wrote:
> > >     I have tried to use mutable with no luck. I doesn't understand how
> > >     can
> > >     I properly apply it in my case: isValidNewName() calls
> > >     sectionNames()
> > >     of its private part, and it calls update() public method. It is
> > >     absolutely unlogic to make update() const function. And also two
> > >     subsequent calls of isValidNewName() could give you different
> > >     results, even if KoSectionManager isn't changed. You can create a
> > >     section between them without referencing anywhere KoSectionManager
> > >     (creating KoSection with QTextCursor, will get document associated
> > >     KoSectionManager by pointer, and update() of it will be called and
> > >     that will be done regardless of you local entity of KoSectionManager
> > >     is const or not (which is pointed)). And KoSectionManager isn't
> > >     supposed to be part of some other class, as I think. It is created
> > >     dynamically and only links to it are stored.>
> > > 
> > > Camilla Boemann wrote:
> > >     ah yes, you are right it can't be applied to this case. Hmmm why do
> > >     sections even have names, and why do they then have to be unique -
> > >     surely we are not exposing some odf attribute?
> 
> 19.835.14 <text:section>
> The text:name attribute specifies a unique name for a section.
> The text:name attribute is usable with the following element: <text:section>
> 5.4.
> The text:name attribute has the data type string 18.2.
> 
> (Note the beautiful typo in the heading where it says text:section instead
> of text:name. :) )
> 
> It does not say that it has to have a name, but if it has, it has to be
> unique.  It does also not say that it does *not* have to have a name, so
> it's unclear whether the name is obligatory or optional.

Wait...

5.4 <text:section>

5.4.1 General
The <text:section> element represents a named region of content in a document.

So I guess it does indeed have to have a name.