[vtk-developers] VTK coding conventions clarification

[David E DeMarle]

+1

David E DeMarle
Kitware, Inc.
R&D Engineer
21 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4909


On Wed, Mar 19, 2014 at 9:10 AM, David Lonie <david.lonie at kitware.com>wrote:

> Sounds like removing the restriction on std::string in Core has the most
> support. Any objections? I'll add std::string support to vtkOStreamWrapper
> and update the conventions document later this week if I don't hear any
> reasons why this shouldn't be done.
>
> Thanks!
> Dave
>
>
> On Tue, Mar 18, 2014 at 11:56 AM, David Gobbi <david.gobbi at gmail.com>wrote:
>
>> No worries about the wrappers, they have all supported std::string
>> just fine ever since VTK 5.8.  And std::string should definitely be
>> preferred over vtkStdString as an interface type.
>>
>>   David
>>
>>
>> On Tue, Mar 18, 2014 at 9:46 AM, Sebastien Jourdain
>> <sebastien.jourdain at kitware.com> wrote:
>> > I'm not sure for Java. When vtkStdString was used, we were getting a
>> > vtkStdString object and not a regular Java String object.
>> > But don't know for std::string...
>> >
>> > Seb
>> >
>> >
>> > On Tue, Mar 18, 2014 at 9:38 AM, David Lonie <david.lonie at kitware.com>
>> > wrote:
>> >>
>> >> On Tue, Mar 18, 2014 at 11:33 AM, Berk Geveci <berk.geveci at kitware.com
>> >
>> >> wrote:
>> >>>
>> >>> Not an answer to your question but a concern. Did you verify that
>> >>> public methods that use std::string are wrapped properly in all
>> >>> languages that we support? If not, you will have provide public
>> >>> methods that use char* in addition. Or add support for std::string to
>> >>> all wrapper generators. I suspect that Python works but I don't know
>> >>> about others.
>> >>
>> >>
>> >> I have not, I assumed that the rule in the guidelines implies that they
>> >> are supported. I can test this out if we decide to keep std::strings
>> around
>> >> in our API.
>> >>
>> >> Does anyone who works on the wrappings know off the top of their heads?
>> >>
>> >> Dave
>> >>
>> >>>
>> >>> On Tue, Mar 18, 2014 at 11:09 AM, David Lonie <
>> david.lonie at kitware.com>
>> >>> wrote:
>> >>> > Hi list,
>> >>> >
>> >>> > Referring to the document here:
>> >>> >
>> >>> > http://public.kitware.com/Wiki/VTK_Coding_Standards
>> >>> >
>> >>> > there are two conventions that appear to be in conflict:
>> >>> >
>> >>> > 27. STL usage is disallowed in the Common modules' public API, and
>> >>> > should
>> >>> > only be used in implementation files there. The other modules may
>> use
>> >>> > STL,
>> >>> > but should do so only when necessary if there is not an appropriate
>> VTK
>> >>> > class. Care should be taken when using the STL in public API,
>> >>> > especially in
>> >>> > the context of what can be wrapped.
>> >>> >
>> >>> > Rationale: limits header inclusion bloat, wrappers are not capable
>> of
>> >>> > handling many non-vtkObject derived classes.
>> >>> >
>> >>> >
>> >>> > and
>> >>> >
>> >>> > 36. Do not use vtkStdString in new API; prefer std::string.
>> >>> >
>> >>> > Rationale: vtkStdString was introduced as a workaround for compilers
>> >>> > that
>> >>> > couldn't handle the long symbol name for the expanded std::string
>> type.
>> >>> > It
>> >>> > is no longer needed on modern platforms.
>> >>> >
>> >>> >
>> >>> > I've added some new API that uses std::string to set some character
>> >>> > string
>> >>> > attributes, but vtkSetMacro and friends all try to pass the value to
>> >>> > the
>> >>> > debug stream, which uses the vtkOStreamWrapper from Common/Core.
>> Since
>> >>> > compilers and standard library implementation have matured since
>> many
>> >>> > of
>> >>> > these rules were put in place, I wonder if it may be time to relax
>> some
>> >>> > of
>> >>> > these restrictions?
>> >>> >
>> >>> > To specifically address my issue of having new API using std::string
>> >>> > that
>> >>> > can't be used with the VTK macros, I see a few different solutions:
>> >>> >
>> >>> > 1) Don't use the macros. This doesn't really address the issue, but
>> >>> > will let
>> >>> > us avoid it for a while.
>> >>> >
>> >>> > 2) Use the vtkSetStringMacro etc that operate char* member
>> variables.
>> >>> > The
>> >>> > explicit memory handling of this approach is annoying, and it's
>> nice to
>> >>> > be
>> >>> > able to use non-C-array string representations that clean up after
>> >>> > themselves, copy/compare as objects, etc.
>> >>> >
>> >>> > 3) Remove rule #27 and allow STL containers everywhere, or at least
>> >>> > make an
>> >>> > exception for std::string if it is to be our preferred string class.
>> >>> >
>> >>> > 4) Remove rule #36 and make vtkStdString the defacto string class
>> for
>> >>> > the
>> >>> > VTK library.
>> >>> >
>> >>> > Any insight would be appreciated :-)
>> >>> >
>> >>> > Dave
>>
>
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://www.vtk.org/mailman/listinfo/vtk-developers
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20140319/a5d233aa/attachment-0002.html>