bug with wxVariant

[Willem Kokke]

On 22/10/06, Sebastien Senechal <seb at cyberdine.ca> wrote:
>
> Hi  Willem,
>
>
> Le mardi octobre 17 2006 18:33, Willem Kokke a =E9crit:
> > wxVariant::GetLong ends up calling Convert
> >
> > bool wxVariant::Convert(long* value) const
> > {
> >     wxString type(GetType());
> >     if (type =3D=3D wxT("double"))
> >         *value =3D (long) (((wxVariantDoubleData*)GetData())->GetValue(=
));
> >     else if (type =3D=3D wxT("long"))
> >         *value =3D ((wxVariantDataLong*)GetData())->GetValue();
> > #ifdef HAVE_BOOL
> >     else if (type =3D=3D wxT("bool"))
> >         *value =3D (long) (((wxVariantDataBool*)GetData())->GetValue());
> > #endif
> >     else if (type =3D=3D wxT("string"))
> >         *value =3D wxAtol((const wxChar*)
> > ((wxVariantDataString*)GetData())->GetValue());
> >     else
> >         return false;
> >
> >     return true;
> > }
> >
> >
> > so it creates 4 temporary string object, loads of string compares and in
> > the end it just calls wxAtol, which calls atol
>
> >
> > so you are creating a extra temporary wxVariant for no good reason,
> wasting
> > memory and processor time, just to save a line of code?
> - yes ! several lines of code multiplied by number of times i used that...
> - alternative is to create wxString temporary variable anyway....
> - i already use atoi for simple and fast conversions :)
> - and I didn't create wxVariant for that purpose, most of my variables are
> Already variant...



generic code, less lines of code, more streamlined and readable code, less
> prone to errors, less debugging, less maintenance, less cost.
> and yes there is a cost to using generic variable, must have some check on
> type at some point...


I suppose I have just a different mind set, since I come from a graphics
programming background, where speed is important.. if I'd use a array of
wxVariants to store data, I'd propably get fired..

also if i followed your suggestions, why use functionnalities like
> wxHashList,
> etc... instead of goo old STL functions hasmap ? waste of memory and
> processor time too ? to save a line of code or 2? :)


I use STL in lots of places, for vectors and hash maps, and i use it instead
of the wxWidgets constructs.


but i am concerned with number of operations ttaken by this convert
> indeed...
> shall i avoid wxWdigets facilities and prefer stl whenever possible?


if you use #DEFINE USE_STL in setup.h, wxString and more is just a wrapper
for the std::(w)string

By the way,  for the exact opposite operation why is wxString(int) still
> private ?
>
>         this is not possible :
>         wxLogVerbose( "there is " + nb + " items containing " + nb2 +
> "other items")
>
>         and again... we need to put brackets and write xxx more lines of
> code..
>         don't u find this inconvenient ? or please tell me how u do...
>         as i use lots of  debug messages ... (cost of performance but hell
> debugging
> is extremelly fast)


 wxString(int) is private intentional, to prevent it ever getting called. it
even says so in the comments :

private:
  // if we hadn't made these operators private, it would be possible to
  // compile "wxString s; s =3D 17;" without any warnings as 17 is implicit=
ly
  // converted to char in C and we do have operator=3D(char)
  //
  // NB: we don't need other versions (short/long and unsigned) as attempt
  //     to assign another numeric type to wxString will now result in
  //     ambiguity between operator=3D(char) and operator=3D(int)
  wxString& operator=3D(int);

  // these methods are not implemented - there is _no_ conversion from int
to
  // string, you're doing something wrong if the compiler wants to call it!
  //
  // try `s << i' or `s.Printf("%d", i)' instead
  wxString(int);

> then you might as well use atol(str.c_str()); and get it over with...
> yup
> >
> > Some people do need to test whether the string contains a valid number.
> I
> > prefer having a concise library instead of a library with hundreds of
> > utility functions that just discard one single parameter.
> I agree with you. i am not asking to change the library :)
> i am still discovering wxWidgets, and i just try to understand the logic
> behind such or such functionnality. i have the impression wxWdigets is
> heavily inspired by MFC and windows programming ...


Are you trying to offend people?  ;-)
I much prefer wxWidgets over  MFC. and since it is heavily cross-platform, I
doubt it is inspired by windows programming.

and i still consider
> char tot =3D wxVariant("255").GetChar();=3D> =3D0x00
> as being a bug, since any developper stubbling upon that code cannot
> 'guess'
> this is not 'allowed' in that case, and will spend some debugging time
> finding out how...


of you try to do that, it will do a wxFAIL_MSG(wxT("Could not convert to a
char")), so the user gets informed

anyway, if you add

    else if (type =3D=3D wxT("string"))
    {
        long l;
        bool success =3D
(((wxVariantDataString*)GetData())->GetValue().ToLong(&l));
        if (success)
        {
            if( (-1<l) && (256>l))
            {
                *value =3D (wxChar)l;
                return true;
            }
        }
        return false;
    }

to wxVariant::Convert(wxChar* value) (lin 1991 in variant.cpp) it should
work..

  also wxVariant not being a conversion utlity is different from other
> languages (i.e. QVariant in QT)


QT is not another language, but another c++ library. there is no reason for
wxWidgets to imitate behaviour from any other language or library
I must admit though that after looking at the variant.cpp sourcecode, it
looks slightly unpolished.

>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.wxwidgets.org/pipermail/wx-users/attachments/20061023/38d=
af0b2/attachment.htm