[Development] [QML] Making tryCompare accept a message

[Alan Alpert]

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.

> 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.

--
Alan Alpert