[Development] [QML] Making tryCompare accept a message

[Albert Astals Cid]

On Thursday 01 August 2013 12:09:26 you wrote:
> On Thu, Aug 1, 2013 at 12:47 AM, Albert Astals Cid
> 
> <albert.astals at canonical.com> wrote:
> > On Wednesday 31 July 2013 09:50:36 Alan Alpert wrote:
> >> On Wed, Jul 31, 2013 at 4:34 AM, Albert Astals Cid
> >> 
> >> <albert.astals at canonical.com> wrote:
> >> > Hi, when using the QML TestLib the compare function has a message
> >> > parameter
> >> > 
> >> >         \qmlmethod TestCase::compare(actual, expected, message = "")
> >> >         
> >> >         Fails the current test case if \a actual is not the same as
> >> >         \a expected, and displays the optional \a message.
> >> > 
> >> > We have found the need for tryCompare to have also the message
> >> > parameter
> >> > since at the moment is not there
> >> > 
> >> >         \qmlmethod TestCase::tryCompare(obj, property, expected,
> >> >         timeout =
> >> >         5000)
> >> > 
> >> > So I was wondering if we should introduce a new function like
> >> > 
> >> >         \qmlmethod TestCase::tryCompareWithMessage(obj, property,
> >> >         expected,
> >> > 
> >> > message = "", timeout = 5000)
> >> > 
> >> > or if we could use javascript trickery to just change the existing
> >> > tryCompare function to
> >> > 
> >> >         \qmlmethod TestCase::tryCompare(obj, property, expected,
> >> >         message =
> >> >         "",
> >> > 
> >> > timeout = 5000)
> >> > 
> >> > and then in the code do (note code is untested but something like that
> >> > should be possible)
> >> > 
> >> >         if (typeof(message) == "number") {
> >> >         
> >> >             // The 4th parameter used to be timeout
> >> >             timeout = msg
> >> >             message = undefined
> >> >         
> >> >         }
> >> > 
> >> > i.e. use Javascript type magic to still let people pass the timeout as
> >> > fourth parameter to keep compatibility with the old signature.
> >> > 
> >> > Personally i favour this second option but if you prefer the first one
> >> > that's fine too.
> >> 
> >> What's the problem with the third option?
> >> \qmlmethod TestCase::tryCompare(obj, property, expected, timeout =
> >> 5000, message = "")
> >> 
> >> Yes, you'll need to specify a timeout sometimes when you don't want
> >> to, but there's a similar problem if you put message first.
> > 
> > Ah, right that one. I did discard this one because it does not keep the
> > "logical ordering" used in compare thus when you decide to change from
> > compare to tryCompare you have not only to split the actual to
> > obj+property but also have to remember to add the timeout method in the
> > middle of your parameter chain.
> 
> The way I see it you're ruining the "logical ordering" the other way,
> because message should come last always. This what seems natural when
> you're writing it as tryCompare from the start, and not just trying to
> port a compare to a tryCompare. But even when you change from compare
> to tryCompare you should be cognizant of the timeout, and this
> ordering is supportive of that.

Makes sense

> 
> > But if you prefer this one it's ok i can a check that makes sure that
> > timeout is not a string for people that may make the mistake i just
> > mentioned.
> A check to help prevent obvious errors is fine, I think we already
> have one for when people port from compare to tryCompare without
> splitting up the actual to obj+property.

Yep i added that one :-)

Ok. so here you go

https://codereview.qt-project.org/#change,62114

Let's move the discussion there :-)

Cheers,
  Albert

> 
> --
> Alan Alpert