Typed lists in docview

[troelsk]

While maintaining bw compatibility. No docs yet - needs to know first if this is the right way to implement typed and exported lists nowadays?

[troelsk]

Trunk

[vadz]

Why use lists instead of wxVector<>?

In any case, it doesn't seem like a good idea to have GetFoos(vector&) for each and every list GetFoos() in wx API. It would be probably more useful to provide a template converter from wxList to wxVector<> that could be used everywhere.

[troelsk]

It would be probably more useful to provide a template converter from wxList to wxVector<> that could be used everywhere.

-1 This will encourage people to keep using the untyped list. wxList is a legacy leftover or?

Why use lists instead of wxVector<>?

+1 wxVector is fine with me, I can make a new patch using it (if it stands a chance of being applied). Esp wxDocList will be very handy to have, really useful in user space, to me at least.

[vadz]

Replying to troelsk:

It would be probably more useful to provide a template converter from wxList to wxVector<> that could be used everywhere.

-1 This will encourage people to keep using the untyped list. wxList is a legacy leftover or?

Yes, it is, but we're not going to be able to get rid of it because there is too much code using it. And there are dozens if not hundreds of functions returning it in wx, so the thought of adding dozens or hundreds new parallel functions mirroring them just doesn't seem appealing.

Why use lists instead of wxVector<>?

+1 wxVector is fine with me, I can make a new patch using it (if it stands a chance of being applied). Esp wxDocList will be very handy to have, really useful in user space, to me at least.

I do agree that it would be nice to have a type-safe and nicer interface for this and other wxLists but I'm not convinced that adding tons of new functions is a good idea to do it.

Perhaps we should add wxList::AsVector()? Then we'd be able to write

const wxVector<wxWindow*> tlws = wxTopLevelWindows.AsVector();
for ( auto& tlw : tlws ) {
  ...
}

[troelsk]

wxList...we're not going to be able to get rid of it because there is too much code using it
it doesn't seem like a good idea to have GetFoos(vector&) for each and every list GetFoos() in wx API

I'm only proposing to this to docview, to retire wxList there.
Using docview takes so much casting currently, this is clearly bad.

And this is no better, it is horrible to impose users to write such cruft, in user space,

const wxVector<wxDocument*> docs = docManager->GetDocuments().AsVector<wxDocument>();

This is better, user does not need to learn about wxList and AsVector() at all

wxVector<wxDocument*> docs;
docManager->GetDocuments(docs);

Okay with GetFoos() in docview?

I'll make a stab at implementing wxList::AsVector(), to use in docview GetFoos().

[troelsk]

wxList.AsVector

[troelsk]

Added a new patch, with wxList::AsVector().

Patch note: I had to remove the wx/utils.h include from vector.h to get list.h to build
(utils.h pulls in GUI stuff, mousestate.h etc)

[vadz]

Some comments about the patch:

1. Calling vectors lists is confusing...
2. Why do Get{Views,Documents,Templates}() take a reference instead of just returning the vectors?
3. In AsVector() it could be useful to call reserve() before populating the vector. Or even preallocating it with the exact number of elements and then using vec[n] = ... instead of push_back().
4. wxCheckCast() is not supposed to be used directly, you should use wxStaticCast(). However both of them only work with classes that use wxRTTI macros and I'm not sure if wxList is always used with objects of these classes.
5. I don't understand the point of MakeTemplateArray() and I really don't like not using wxScopedArray for it.

[troelsk]

AsSafeVector

[troelsk]

Okay. Thanks for feedback.

1. Calling vectors lists is confusing...

Ok, changed to wx{View,Doc,Template}Vector

2. Why do Get{Views,Documents,Templates}() take a reference instead of just returning the vectors?

Done, these method names are taken, cannot overload by return type only so names now changed to Get{View,Document,Template}Vector()

3. In AsVector() it could be useful to call reserve() before populating the vector...

Done

4. wxCheckCast() is not supposed to be used directly, you should use wxStaticCast()...wxRTTI macros

Ok, addressing these two issues raised by having two methods, AsVector and AsSafeVector.

5. I don't understand the point of MakeTemplateArray() and I really don't like not using wxScopedArray for it.

&templates[0] looks suspect to me, is this really working/correct/safe? Else, MakeTemplateArray() can be removed

BTW, wxScopedArray has no copy ctor

    wxScopedArray<wxDocTemplate*> templateArray(new wxDocTemplate*[list.size()]);
...
    return templateArray; // docview.cpp(1418) : error C2248: 'wxScopedArray<T>::wxScopedArray' : cannot access private member declared in class 'wxScopedArray<T>'

[vadz]

Replying to troelsk:

&templates[0] looks suspect to me, is this really working/correct/safe?

If templates is a vector, yes. Vector is guaranteed to store its items in contiguous memory.

BTW, wxScopedArray has no copy ctor

There is no good (i.e. not std::auto_ptr<>-like) solution to this in C++98.

[troelsk]

Removed MakeTemplateArray() and applied vector in some looping code

[troelsk]

Ok. New patch. Removed MakeTemplateArray() and applied vector in some looping code. If this is to be applied I guess I need to make some documentation?

[vadz]

Sorry...

I don't like AsSafeVector() thing. If we really need it, we should use some template helper applying either simple static_cast<> or wxStaticCast() depending on whether the latter can be used.

Also, why all the changes to the implementation code? I mean, they're nice and all but hardly necessary. And they actually make the code less efficient. And break its compilation with VC6 because these new functions can't be compiled with it (it doesn't support explicitly choosing the function template specialization to call, see comment near wxCheckCast() definition).

So why not just keep this very simple and add:

1. A single AsVector<>.
2. The vector-returning accessors (inside #ifndef __VISUALC6__ to not have to deal with that compiler).
3. Documentation for them.

And not change anything else?

[troelsk]

Ok, added new minimal patch, with a little documentation.
(What I really wants from this ticket is actually just the wxDocVector typedef which I'd rather see declared in wx than in some header in my user code)
Thanks

[troelsk]

Minimal version

[VZ]

(In [73002]) Don't include wx/utils.h from wx/vector.h.

This will allow to include wx/vector.h from wx/list.h which is itself included
from wx/utils.h by breaking this circular dependency.

Don't use wxMin(), defined in wx/utils.h, in order to do this.

See #14814.

[VZ]

(In [73003]) Add wxList::AsVector<>() helper.

This can be useful in legacy code using wxList to progressively replace it
with wxVector.

See #14814.

[VZ]

(In [73004]) Add wxDocManager::Get{Views,Documents,Templates}Vector().

Add accessors returning more convenient wxVectors to supplement the existing
ones giving access to internally used wxLists.

Closes #14814.

[troelsk]

Great, thanks, already put to good use,
 http://wxedit.svn.sourceforge.net/viewvc/wxedit/include/wx/ext/wx.h?r1=135&r2=134&pathrev=135

[troelsk]

inline

[troelsk]

[thread.gmane.org/gmane.comp.lib.wxwidgets.devel/145412]

return m_templates.AsVector<wxDocTemplate*>();

Perhaps the compiler stops reporting errors in template code after the
first one.

Nope, if I comment out the first one it does nor report on the 2 others.

Apparantly this compiler needs the full declaration, see new patch "inline.patch"

[troelsk]

Apparantly this compiler needs the full [wxView class] declaration

[VZ]

(In [73048]) Compilation fix for wxDocManager after r73004.

Define GetXXXVector() methods after all the classes are fully declared to
ensure that static_cast<> inside wxList::AsVector() they use compiles with the
OpenVMS compiler.

See #14814.