You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flex.apache.org by Justin Mclean <ju...@classsoftware.com> on 2014/03/11 07:23:18 UTC
ADG Bug of the day
Hi,
Anyone see anything wrong with this method (from the ADG)?
mx_internal function columnWordWrap(c:AdvancedDataGridColumn):Boolean
{
if (c.wordWrap == true)
return true;
if (c.wordWrap == false)
return false;
return wordWrap;
}
Turns out wordWrap is a * and should only have the values true, false and undefined. Who would like to bet that it may be set to other values? :-)
Thanks,
Justin
Re: ADG Bug of the day
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I don't know how much you'll actually save after the JIT
No idea either - plus it very likely this is not exactly a bottle neck in the ADG code.
But the original code did offend me as it's meaning was IMO less than clear.
Thanks,
Justin
Re: ADG Bug of the day
Posted by Alex Harui <ah...@adobe.com>.
Not surprising. The question is whether you truly care if null is
supposed to be a separate value than undefined.
On 3/11/14 12:17 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> I won't veto it (unless you use == instead of ===). I think
>> all undefined checks should use ===. Otherwise it would match null.
>
>JFYI There probably 100+ places in the SDK were we use == undefined
>rather than == undefined. From a quick search it look like === undefined
>is used is a few more places.
>
>There's a simple patch if someone want to test and submit one :-)
>
>Thanks,
>Justin
Re: ADG Bug of the day
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I won't veto it (unless you use == instead of ===). I think
> all undefined checks should use ===. Otherwise it would match null.
JFYI There probably 100+ places in the SDK were we use == undefined rather than == undefined. From a quick search it look like === undefined is used is a few more places.
There's a simple patch if someone want to test and submit one :-)
Thanks,
Justin
RE: ADG Bug of the day
Posted by Maurice Amsellem <ma...@systar.com>.
>I think all undefined checks should use ===.
Thanks Alex for the tip, I didn't know that.
I think my code needs some review ;-)
Maurice
-----Message d'origine-----
De : Alex Harui [mailto:aharui@adobe.com]
Envoyé : mardi 11 mars 2014 08:00
À : dev@flex.apache.org
Objet : Re: ADG Bug of the day
There are plenty of places to improve SDK code. If that's your itch, start scratching. I don't know how much you'll actually save after the JIT, but hey, I won't veto it (unless you use == instead of ===). I think all undefined checks should use ===. Otherwise it would match null.
-Alex
On 3/10/14 11:51 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>And give the default is undefined it make sense to make that the faster
>path IMO.
>
>Am I missing something? Do you think otherwise?
>
>Thanks,
>Justin
Re: ADG Bug of the day
Posted by Alex Harui <ah...@adobe.com>.
There are plenty of places to improve SDK code. If that's your itch,
start scratching. I don't know how much you'll actually save after the
JIT, but hey, I won't veto it (unless you use == instead of ===). I think
all undefined checks should use ===. Otherwise it would match null.
-Alex
On 3/10/14 11:51 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>And give the default is undefined it make sense to make that the faster
>path IMO.
>
>Am I missing something? Do you think otherwise?
>
>Thanks,
>Justin
Re: ADG Bug of the day
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
And give the default is undefined it make sense to make that the faster path IMO.
Am I missing something? Do you think otherwise?
Thanks,
Justin
Re: ADG Bug of the day
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> At Adobe, the policy was GIGO (Garbage In, Garbage Out). If you provide
> values outside of the allowed values, we did not promise anything
> reasonable would happen and we generally did no check the values assigned.
> Do we want to switch to a policy to fatten and slow down the SDK with
> input checking?
In this case doing a check against undefined is probable faster and IMO more clearly states what is going on.
if (c.headerWordWrap == undefined)
return headerWordWrap;
else
return c.headerWordWrap;
That's one check and one property lookup if it's undefined or one check and two property lookups if not.
if (c.wordWrap == true)
return true;
if (c.wordWrap == false)
return false;
return wordWrap;
That's two checks and two property lookups if it's undefined or one or two checks and one or two property lookups if not.
Thanks,
Justin
Re: ADG Bug of the day
Posted by Alex Harui <ah...@adobe.com>.
It is (was) common to use * for a three-value boolean where undefined
means "not-specified, check somewhere else"
On 3/11/14 3:08 AM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>> I would say to use a proper data type. Would like to think we didn't
>>blindly use * for things just for the sake of convenience.
>That was my very first though but it's part of a public API so we can't
>easily change that.
>
>Thanks,
>Justin
Re: ADG Bug of the day
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> I would say to use a proper data type. Would like to think we didn't blindly use * for things just for the sake of convenience.
That was my very first though but it's part of a public API so we can't easily change that.
Thanks,
Justin
RE: ADG Bug of the day
Posted by Kessler CTR Mark J <ma...@usmc.mil>.
I would say to use a proper data type. Would like to think we didn't blindly use * for things just for the sake of convenience.
-Mark
-----Original Message-----
From: Alex Harui [mailto:aharui@adobe.com]
Sent: Tuesday, March 11, 2014 2:32 AM
To: dev@flex.apache.org
Subject: Re: ADG Bug of the day
At Adobe, the policy was GIGO (Garbage In, Garbage Out). If you provide
values outside of the allowed values, we did not promise anything
reasonable would happen and we generally did no check the values assigned.
Do we want to switch to a policy to fatten and slow down the SDK with
input checking?
On 3/10/14 11:23 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>Anyone see anything wrong with this method (from the ADG)?
>
> mx_internal function columnWordWrap(c:AdvancedDataGridColumn):Boolean
> {
> if (c.wordWrap == true)
> return true;
> if (c.wordWrap == false)
> return false;
>
> return wordWrap;
> }
>
>Turns out wordWrap is a * and should only have the values true, false and
>undefined. Who would like to bet that it may be set to other values? :-)
>
>Thanks,
>Justin
Re: ADG Bug of the day
Posted by Alex Harui <ah...@adobe.com>.
At Adobe, the policy was GIGO (Garbage In, Garbage Out). If you provide
values outside of the allowed values, we did not promise anything
reasonable would happen and we generally did no check the values assigned.
Do we want to switch to a policy to fatten and slow down the SDK with
input checking?
On 3/10/14 11:23 PM, "Justin Mclean" <ju...@classsoftware.com> wrote:
>Hi,
>
>Anyone see anything wrong with this method (from the ADG)?
>
> mx_internal function columnWordWrap(c:AdvancedDataGridColumn):Boolean
> {
> if (c.wordWrap == true)
> return true;
> if (c.wordWrap == false)
> return false;
>
> return wordWrap;
> }
>
>Turns out wordWrap is a * and should only have the values true, false and
>undefined. Who would like to bet that it may be set to other values? :-)
>
>Thanks,
>Justin