Issues in the javascript library

Post here your questions about the HTML5 / JavaScript for SFS2X

Moderators: Lapo, Bax

jdv145
Posts: 21
Joined: 11 May 2013, 13:13

Issues in the javascript library

Postby jdv145 » 16 Oct 2013, 17:54

Hello all,

I was wondering if the person(s) who made the javascript api were aware that it is not recommended to loop over an array using a for..in construct. One of the first google hits about this is http://stackoverflow.com/q/500504/346992

The reason why i mention this is because i noticed some bugs popping up in my javascript-console. Ii looked in the source of SFS2X_API_JS.js to see if i could fix it. I noticed a for..in loop was used for looping over an array, so i decided to quickly change all for..in loops structures with a regexp to fix this (by checking if the object being looped over was instanceof Array). A day later i got a new bug caused by my 'fix'. Again i did some snooping in the code and i noticed an empty array was used to store key/val's (just like you would do with an object).

I have a gut feeling it's an issue for the whole library, but if not, i can provide some more details and the exact changes i did .

Regards,

Jan

PS client API: JS Version: 1.0.3
server: Version: 2.7.0
jdk7
win7 x64
User avatar
Bax
Site Admin
Posts: 4609
Joined: 29 Mar 2005, 09:50
Location: Italy
Contact:

Re: Issues in the javascript library

Postby Bax » 17 Oct 2013, 08:40

The post @ stackoverflow you mentioned doesn't say that for-in loops shouldn't be used: it just explains the differences between iterating over an array by numeric indexes or by key. If you know what you are doing then there's no problem in using that construct. With your "fix" is is highly likely that you broke the API functioning, and in fact you caused other bugs to show up.
What you should have done since from the beginning was to report the initial bugs popping up in the console in order to discuss them, not to try a supposed fix in the (obfuscated) API code.
Paolo Bax
The SmartFoxServer Team
jdv145
Posts: 21
Joined: 11 May 2013, 13:13

Re: Issues in the javascript library

Postby jdv145 » 17 Oct 2013, 11:37

I agree. If i wasn't busy working to keep a deadline i wouldn't have done a fix myself, but would have posted it immediately. My change was only temporary, to keep the work going, and consisted only of a wrapper function on the object on which the loop is on. The only thing my fix did was to copy the 0..n keys to a seperate object to iterate over, since i thought that was the intention. But then i got an error somewhere else much later and noticed an empty array was being used also as a key/val object .

Although the javascript is obfuscated when its downloaded, for development i'm using a version which has been expanded by my editor... and then it's pretty readable when looking at an error. The variable names are gone, but with a good indentation its doable to try to find the intent of the code and why it breaks in case of an error.

I looked into it again to see why exactly it is going wrong. I tried one of the examples of the API and the error popped up immediately (the example from SFS2X.Requests.System.SetUserVariablesRequest).

After closer inspection i noticed the for loops iterated not only on the keys, but also some prototyped functions as it is explained here: http://stackoverflow.com/q/1107681/346992 . I don't think its absolutely wrong to use for..in, but it wouldn't be my personal choice of doing it for a library used by others.

The deliberate usage of this sort of structure, implies an assumption that no user of smartfox will make use directly or indirectly (by importing a library) of prototypes on arrays. No problem... if you are aware of this.

PS i'm not a fan of prototyped function either
User avatar
Bax
Site Admin
Posts: 4609
Joined: 29 Mar 2005, 09:50
Location: Italy
Contact:

Re: Issues in the javascript library

Postby Bax » 18 Oct 2013, 08:25

We don't use prototypes on arrays.

I looked into it again to see why exactly it is going wrong. I tried one of the examples of the API and the error popped up immediately (the example from SFS2X.Requests.System.SetUserVariablesRequest).

Can you please describe what error do you get?
Paolo Bax
The SmartFoxServer Team
jdv145
Posts: 21
Joined: 11 May 2013, 13:13

Re: Issues in the javascript library

Postby jdv145 » 18 Oct 2013, 12:58

If you had used prototypes on Array's you would have seen the error also, so i assumed you guys didn't. Just declare one and run the SetUserVariablesRequest example from the api and you will see what i mean.

Part of the stack trace below:

Code: Select all

TypeError: Object function (num, val) {[*removed*]} has no method 'toArray'
    at d.SFS2X.Requests.System.SetUserVariablesRequest.SFS2X.Requests._BaseRequest.extend.execute (http://localhost/gameportal/js/SFS2X_API_JS3.js:2229:43)
    at SFS2X.SmartFox.send (http://localhost/gameportal/js/SFS2X_API_JS3.js:88:37)
 


The block of code this refers to below. Look at the <==== to see the problematic line.

Code: Select all

 
 SFS2X.Requests.System.SetUserVariablesRequest = SFS2X.Requests._BaseRequest.extend({init: function(a) {
        this._super(SFS2X.Requests.SetUserVariables);
        this._userVariables = a
    }, validate: function() {
        var a = [];
        (null == this._userVariables || 0 == this._userVariables.length) && a.push("No variables were specified");
        if (0 < a.length)
            throw new SFS2X.Exceptions.SFSValidationError("SetUserVariablesRequest Error", a);
    }, execute: function() {
        var a = [], b;
        for(b in loopObj(this._userVariables))
            a.push(this._userVariables[b].toArray()); //<======= this is the error line 2229
        this._reqObj[this.constructor.KEY_VAR_LIST] =
        a
    }});
User avatar
Bax
Site Admin
Posts: 4609
Joined: 29 Mar 2005, 09:50
Location: Italy
Contact:

Re: Issues in the javascript library

Postby Bax » 18 Oct 2013, 13:29

This is the original code:
execute: function(sfs)
{
var varList = [];

for (var u in this._userVariables)
{
var userVar = this._userVariables[u];
varList.push(userVar.toArray());
}

this._reqObj[this.constructor.KEY_VAR_LIST] = varList;
}

The toArray() method is called on the object userVar which is a SFS2X.Entities.Variables.SFSUserVariable.
I still don't get what the problem is.
Paolo Bax
The SmartFoxServer Team
jdv145
Posts: 21
Joined: 11 May 2013, 13:13

Re: Issues in the javascript library

Postby jdv145 » 18 Oct 2013, 14:41

Bax wrote:
The toArray() method is called on the object userVar which is a SFS2X.Entities.Variables.SFSUserVariable.
I still don't get what the problem is.


The thing is that when using

Code: Select all

for(b in this._userVariables)


You expect the b to go like
"0", "1", "2", ... etc for every uservariable.

What happens is that b goes like

"0", "1", "2",..., "first", "last", "foo"

because somewhere in a part of javascript 'miles away' which has nothing to do with smartfox there is a

Code: Select all

Array.prototype.first = function() {}
Array.prototype.last= function() {}
Array.prototype.foo= function() {}


Then inside the loop this code will trigger an error:

Code: Select all

this._userVariables['foo'].toArray()


because this._userVariables['foo'] is a function which has no toArray() function and hence

Code: Select all

TypeError: Object function (num, val) {[*removed*]} has no method 'toArray'


NB the order of the keys might differ, since by specification there is no ordering in the keys of an object.
PS i program in many many languages and since i cant remember every detail on every language i might be incorrect on details but this is what i have deduced.

PPS i use the following code to trigger a warning

Code: Select all

for(var key in []) {
        console.log("@@ Warning, issue might arise in smartfox because of Array.prototype." + key);
        break;
    }
jdv145
Posts: 21
Joined: 11 May 2013, 13:13

Re: Issues in the javascript library

Postby jdv145 » 22 Oct 2013, 15:26

I might have found an other bug.

in the function SFS2X.Controllers.SystemController.prototype._fnInviteUsers

there is a line

a = new SFS2X.Entities.Invitation.SFSInvitation(c, sfs.mySelf, a[SFS2X[...]

which i guess should be

a = new SFS2X.Entities.Invitation.SFSInvitation(c, this._sfs.mySelf, a[SFS2X[...]

Do you guys test with a globally scoped sfs coincidentally ? I got an error about the sfs, but again i'm not entirely sure.
User avatar
Bax
Site Admin
Posts: 4609
Joined: 29 Mar 2005, 09:50
Location: Italy
Contact:

Re: Issues in the javascript library

Postby Bax » 23 Oct 2013, 08:23

You are right, this is a bug. Thank you for pointing out. This will be fixed in the next release.
If possible we will also add a check in the for-in loops to make sure only the elements of the right type are processed (like it already happens in some parts of the code... it seems we missed the check in a few cases).
Paolo Bax
The SmartFoxServer Team
User avatar
Bax
Site Admin
Posts: 4609
Joined: 29 Mar 2005, 09:50
Location: Italy
Contact:

Re: Issues in the javascript library

Postby Bax » 23 Oct 2013, 12:50

Please contact us by email, so we can send you a pre release version.
Paolo Bax
The SmartFoxServer Team

Return to “SFS2X HTML5 / JavaScript API”

Who is online

Users browsing this forum: No registered users and 18 guests