You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flex.apache.org by ah...@apache.org on 2017/06/06 06:56:25 UTC
[2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks like
we get '' (empty string) sometimes which coerces to 0. We want it to be NaN
looks like we get '' (empty string) sometimes which coerces to 0. We want it to be NaN
Project: http://git-wip-us.apache.org/repos/asf/flex-asjs/repo
Commit: http://git-wip-us.apache.org/repos/asf/flex-asjs/commit/44e236a7
Tree: http://git-wip-us.apache.org/repos/asf/flex-asjs/tree/44e236a7
Diff: http://git-wip-us.apache.org/repos/asf/flex-asjs/diff/44e236a7
Branch: refs/heads/release0.8.0
Commit: 44e236a7cdbabb7e3b1609fe66297a97d701dc9c
Parents: c5fa72c
Author: Alex Harui <ah...@apache.org>
Authored: Mon Jun 5 23:49:39 2017 -0700
Committer: Alex Harui <ah...@apache.org>
Committed: Mon Jun 5 23:49:39 2017 -0700
----------------------------------------------------------------------
.../projects/Basic/src/main/flex/org/apache/flex/core/UIBase.as | 4 ++++
1 file changed, 4 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/flex-asjs/blob/44e236a7/frameworks/projects/Basic/src/main/flex/org/apache/flex/core/UIBase.as
----------------------------------------------------------------------
diff --git a/frameworks/projects/Basic/src/main/flex/org/apache/flex/core/UIBase.as b/frameworks/projects/Basic/src/main/flex/org/apache/flex/core/UIBase.as
index d0e8528..b3d9ba6 100644
--- a/frameworks/projects/Basic/src/main/flex/org/apache/flex/core/UIBase.as
+++ b/frameworks/projects/Basic/src/main/flex/org/apache/flex/core/UIBase.as
@@ -371,6 +371,8 @@ package org.apache.flex.core
var strpixels:String = positioner.style.width as String;
if (strpixels !== null && strpixels.indexOf('%') != -1)
pixels = NaN;
+ else if (strpixels.length == 0)
+ pixels = NaN;
else
pixels = parseFloat(strpixels);
if (isNaN(pixels)) {
@@ -463,6 +465,8 @@ package org.apache.flex.core
var strpixels:String = positioner.style.height as String;
if (strpixels !== null && strpixels.indexOf('%') != -1)
pixels = NaN;
+ else if (strpixels.length == 0)
+ pixels = NaN;
else
pixels = parseFloat(strpixels);
if (isNaN(pixels)) {
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks like we get '' (empty string) sometimes which coerces to 0. We want it to be NaN
Posted by Harbs <ha...@gmail.com>.
Changed.
> On Jun 6, 2017, at 1:37 PM, Harbs <ha...@gmail.com> wrote:
>
> True.
>
> else if (strpixels.length == 0)
>
> should probably be:
> else if (strpixels === “”)
>
>> On Jun 6, 2017, at 1:01 PM, Justin Mclean <ju...@classsoftware.com> wrote:
>>
>> Hi,
>>
>>> Language.as() is not being called (@flexjsignorecoercion is being used).
>>
>> And it will still throw an RTE if strpixels is null.
>>
>>> if (strpixels !== null && strpixels.indexOf('%') != -1)
>>
>> will be false
>>
>>> else if (strpixels.length == 0)
>>
>> throws RTE
>>
>> Thanks,
>> Justin
>
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks like we get '' (empty string) sometimes which coerces to 0. We want it to be NaN
Posted by Harbs <ha...@gmail.com>.
True.
else if (strpixels.length == 0)
should probably be:
else if (strpixels === “”)
> On Jun 6, 2017, at 1:01 PM, Justin Mclean <ju...@classsoftware.com> wrote:
>
> Hi,
>
>> Language.as() is not being called (@flexjsignorecoercion is being used).
>
> And it will still throw an RTE if strpixels is null.
>
>> if (strpixels !== null && strpixels.indexOf('%') != -1)
>
> will be false
>
>> else if (strpixels.length == 0)
>
> throws RTE
>
> Thanks,
> Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> Language.as() is not being called (@flexjsignorecoercion is being used).
And it will still throw an RTE if strpixels is null.
> if (strpixels !== null && strpixels.indexOf('%') != -1)
will be false
> else if (strpixels.length == 0)
throws RTE
Thanks,
Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks like we get '' (empty string) sometimes which coerces to 0. We want it to be NaN
Posted by Harbs <ha...@gmail.com>.
Language.as() is not being called (@flexjsignorecoercion is being used).
> On Jun 6, 2017, at 12:50 PM, Justin Mclean <ju...@classsoftware.com> wrote:
>
> Hi,
>
> In more detail the issue is that the property is likely to default to undefined and casting that to String in JS like so org.apache.flex.utils.Language.as(undefined, String)) will set the value to null and strpixels.length with throw a RTE. So it’s likely this change in will cause all sort of issues.
>
> Thanks,
> Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
In more detail the issue is that the property is likely to default to undefined and casting that to String in JS like so org.apache.flex.utils.Language.as(undefined, String)) will set the value to null and strpixels.length with throw a RTE. So it’s likely this change in will cause all sort of issues.
Thanks,
Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
in fact this code will throw a RTE is width / strpixels is null so you may want to reconsider the implementation.
Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks like we get '' (empty string) sometimes which coerces to 0. We want it to be NaN
Posted by Harbs <ha...@gmail.com>.
It sounds like a micro-optimization, but it saves some bytes and doesn’t look like a functional change, so go knock yourself out… ;-)
> On Jun 6, 2017, at 1:43 PM, Justin Mclean <ju...@classsoftware.com> wrote:
>
> Hi,
>
> Just profiled these with a variety of inputs and the first is around 3 times as fast as the second.
>
> I added a null check to stop the RTE, if the null check is not needed it can be removed from both.
>
> public function test(str:String):Number {
> var pixels:Number = NaN;
> if (str !== null && str.indexOf('%') === -1)
> pixels = parseFloat(str);
>
> return pixels;
> }
>
> public function test2(str:String):Number {
> var pixels:Number;
> if (str !== null && str.indexOf('%') != -1)
> pixels = NaN;
> else if (str !== null && str.length == 0)
> pixels = NaN;
> else
> pixels = parseFloat(str);
>
> return pixels;
> }
>
> Thanks,
> Justin
>
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
Just profiled these with a variety of inputs and the first is around 3 times as fast as the second.
I added a null check to stop the RTE, if the null check is not needed it can be removed from both.
public function test(str:String):Number {
var pixels:Number = NaN;
if (str !== null && str.indexOf('%') === -1)
pixels = parseFloat(str);
return pixels;
}
public function test2(str:String):Number {
var pixels:Number;
if (str !== null && str.indexOf('%') != -1)
pixels = NaN;
else if (str !== null && str.length == 0)
pixels = NaN;
else
pixels = parseFloat(str);
return pixels;
}
Thanks,
Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to
be NaN
Posted by piotrz <pi...@gmail.com>.
I think it would be great! Let see what Alex thing.
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-2-4-git-commit-flex-asjs-refs-heads-release0-8-0-looks-like-we-get-empty-string-sometimes-which-cN-tp62159p62168.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> var strpixels:String = element.style.width as String;
> if (strpixels.indexOf('%') === -1)
>
> We may have still RTE ?
Yep it would also RTE if strpixels was null.
So perhaps this instead?
> if (strpixels !== null && strpixels.indexOf('%') === -1)
Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to
be NaN
Posted by piotrz <pi...@gmail.com>.
I think it would be good to change all those with your proposition. For me
it's cleaner, but with this one from you:
var strpixels:String = element.style.width as String;
if (strpixels.indexOf('%') === -1)
We may have still RTE ?
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-2-4-git-commit-flex-asjs-refs-heads-release0-8-0-looks-like-we-get-empty-string-sometimes-which-cN-tp62159p62166.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
> You propose changing whole this part [1] with yours ?
Or something similar / along those line that was off the top of my head and had not been tested.
Perhaps if null is never passed in then it doesn’t need to be checked for and the code may not RTE?
Thanks,
Justin
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to
be NaN
Posted by piotrz <pi...@gmail.com>.
Justin,
You propose changing whole this part [1] with yours ?
[1] https://paste.apache.org/L1Mq
Just wanted to understand.
Thanks,
Piotr
-----
Apache Flex PMC
piotrzarzycki21@gmail.com
--
View this message in context: http://apache-flex-development.2333347.n4.nabble.com/Re-2-4-git-commit-flex-asjs-refs-heads-release0-8-0-looks-like-we-get-empty-string-sometimes-which-cN-tp62159p62164.html
Sent from the Apache Flex Development mailing list archive at Nabble.com.
Re: [2/4] git commit: [flex-asjs] [refs/heads/release0.8.0] - looks
like we get '' (empty string) sometimes which coerces to 0. We want it to be
NaN
Posted by Justin Mclean <ju...@classsoftware.com>.
Hi,
This is probably a simpler / less costly way of doing the same thing:
var pixels:Number = NaN;
var strpixels:String = element.style.width as String;
if (strpixels.indexOf('%') === -1)
pixels = parseFloat(strpixels);
Justin