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
Issues in the javascript library
Re: Issues in the javascript library
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.
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
The SmartFoxServer Team
Re: Issues in the javascript library
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
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
Re: Issues in the javascript library
We don't use prototypes on arrays.
Can you please describe what error do you get?
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
The SmartFoxServer Team
Re: Issues in the javascript library
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:
The block of code this refers to below. Look at the <==== to see the problematic line.
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
}});
Re: Issues in the javascript library
This is the original code:
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.
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
The SmartFoxServer Team
Re: Issues in the javascript library
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;
}
Re: Issues in the javascript library
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.
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.
Re: Issues in the javascript library
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).
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
The SmartFoxServer Team
Re: Issues in the javascript library
Please contact us by email, so we can send you a pre release version.
Paolo Bax
The SmartFoxServer Team
The SmartFoxServer Team
Return to “SFS2X HTML5 / JavaScript API”
Who is online
Users browsing this forum: No registered users and 22 guests