[CMake] Re: Changing MD -> MT (+patch for free toolkit)
Mathieu Malaterre
mathieu.malaterre at gmail.com
Fri Feb 22 05:19:25 EST 2008
On Tue, Feb 19, 2008 at 5:42 PM, Gonzalo Garramuño
<ggarra at advancedsl.com.ar> wrote:
> Mathieu Malaterre wrote:
> > On Feb 19, 2008 5:43 AM, Gonzalo Garramuño <ggarra at advancedsl.com.ar> wrote:
> >> Mathieu Malaterre wrote:
> >>> Ok this was yet-another-weird-dll thingy, one cannot do:
> >>>
> >>> class GDCM_EXPORT String : std::string { ... }
> >>>
> >> Please, learn a little bit more about C++.
> >
> > Oh thank you ! I'll take your advice right away, mister expert. So
> > could you please let me know why *in my particular case* this is
> > wrong:
> > 1. I did not add any data member
> > 2. I did not overload the dstor
> >
>
> GDCM_EXPORT tells me you are using this in a library.
>
> The reason this is wrong is this. Say you write a function to get a
> exension of a filename as uppercase:
>
> GDCM_EXPORT MyString get_extension( const MyString& b )
> {
> MyString ext = b.to_upper();
> // etc... do some other stuff like find '.' and substr
> return ext;
> }
>
> Now... this will work. But.... now func() can only be used with your
> particular MyString class. However, if you had done:
>
> namespace str {
> // to_upper implementation not shown
> GDCM_EXPORT std::string to_upper( const std::string& b );
>
> GDCM_EXPORT std::string get_extension( const std::string& b )
> {
> std::string ext = to_upper( b );
> // etc... do some other stuff like find '.' and substr
> return ext;
> }
> }
My gdcm::String is not a general purpose string, use std::string instead.
> With this, the code for both to_upper() and get_extension() can be
> safely be reused and works for *all* std::string objects. You don't
> force people to use your own string library. This makes your library
> more attractive and easier to reuse in other code.
I'll see about that 'attractiveness' in sf.net stats, thanks. As a
side note you do realize you are using a program that has exactly this
implementation, and seems to be to be quite popular.
> Even, if you did not do 1 or 2, you might have done 3:
> 3. Yo might have overloaded (replaced) a string function.
>
> class MyString : public std::string
> {
> public:
> // silly example, but same applies to add, operator<<, etc.
> size_type size() { return std::string::size() + 1; }
> };
>
> int main()
> {
> MyString* s = new MyString;
> std::string* c = static_cast< std::string* >( s ); // correct
> std::cerr << c->size() << std::endl; // which size() gets called?
> // not yours,
> // but the one in std::string
> return 0;
> }
>
> This example for 3) is very simple and obvious but in big programs this
> issue may be more confusing as the distintion of what a pointer really
> points to becomes more blurry.
I do not overload any member function, I only define operator, they do
not carry any RTTI info, or add function with non-existent signature
in superclass.
> If you have only added non-conflicting functions in MyString to
> std::string, what you have is safe but potentially not so beneficial.
> You might either do as I showed and place your additional functions in a
> namespace and keep using std::string everywhere (which makes your
> library nice to use and probably popular) or create your own string
> class, without deriving anything (and yes, you do need to create a lot
> of code for doing a good string class -- it is C++ after all), or create
> a proxy string class thru containment of std::string (which is like
> creating your own string class from scratch, but a tad simpler):
>
> class MyString
> {
> std::string str;
>
> public:
> typedef std::string::size_type size_type;
> // other typedefs similar to std::string...
>
> public:
> MyString() {}
> MyString(const MyString& b) : str(b.str) {}
> MyString(const std::string& b) : str(b) {}
>
> // auto cast to string - optional (only if lazy, may not be wanted)
> operator std::string() const { return str; }
You are doing a copy.
> // dispatch size function
> inline size_type size() const { return str.size(); }
you are missing the gazillion std::string member functions.
> // my functions - not shown
> void to_upper();
>
> // .... dispatch others, etc...
> // also remember to add operator=(const std::string& b)
> };
>
> This avoids you having to actually write your (potentially buggy) code
My code is 10 lines long, the most difficult part was to redo the typedef.
> to the string functions, as you just dispatch the operations to the
> std::string inside your class. And the operator to std::string() and
> copy constructor of std::string allows your class to interact with
> std::string functions and elements rather seamlessly. It is still
> verbose, but not *THAT* much. And the code is probably safer to use
> through pointers and easy to extend without conflicts.
I'll make sure doxygen does not show the parent class of this
particular class, to avoid such hairy user misuse.
> > Please do not copy some random comment found on the internet and
> > instead comment on my particular implementation with the following 1
> > and 2 restrictions. Thanks.
> >
>
> It is not some "random comment". It is good design advice that sadly
> applies to all C++ STL classes.
>
> For more information, refer to a good C++ book. IIRCC, Scott Meyer's
> Effective STL and Effective C++, covers this and other problems with all
> the pitfalls in a very thorough fashion.
Ok not so random, but taken from a C++ book, right. I have taken good
notes of your advices, but -sadly- they do not apply to my limited
test case.
As a side note, this whole thread kills the only important thing: the
cmake part. My patch was lost in the middle of those comments, so I
uploaded it again on the cmake bug tracker:
http://vtk.org/Bug/view.php?id=6417
On different subject I also uploaded a patch for FindJNI: make
interface more consistent with other module.
--
Mathieu
More information about the CMake
mailing list