Unexpected Token - but I can't see it

Discussion in 'jQuery' started by sarahk, Nov 9, 2015.

  1. #1
    I might be looking too hard :)

    upload_2015-11-10_15-5-12.png

    Just in case the error is actually in my function - can anyone see what the problem is?

    
    function displayMembers(membership_id){
                console.log(membership_id);
                $('#membershipmembers').show();
    
                $.ajax({
                    dataType: "json",
                    url: '/individuals/jsonGetMembers/',
                    data: {id: membership_id},
                    success: processMemberData
                });
            }
    
            function processMemberData(data, textStatus, xhr){
                $("#memberslist").empty();
    
                var statustypes = {'m', 'g', 'x'};
    
                for(var j = 0; j < statustypes.length; j++){
                    for(var i = 0; i < data.length; i++) {
                        var obj = data[i];
                        if (obj.Individual.status == statustypes[j]){
                            addMemberToList(obj);
                        }
                    }
                }
            }
    Code (markup):
     
    sarahk, Nov 9, 2015 IP
  2. sarahk

    sarahk iTamer Staff

    Messages:
    28,788
    Likes Received:
    4,528
    Best Answers:
    123
    Trophy Points:
    665
    #2
    So part of my problem was that I created my array with curly braces and not square. So now I have an unexpected string error

    upload_2015-11-10_16-7-52.png
     
    sarahk, Nov 9, 2015 IP
  3. sarahk

    sarahk iTamer Staff

    Messages:
    28,788
    Likes Received:
    4,528
    Best Answers:
    123
    Trophy Points:
    665
    #3
    So, I've just been reminded why javascript drives me nuts.

    processMemberData calls addMemberToList
    addMemberToList calls another function

    and it was that function that had the error but it was showing at the top of the line, rather than where the error was. That must make life hard for developers who do lots in javascript!
     
    sarahk, Nov 9, 2015 IP
  4. mmerlinn

    mmerlinn Prominent Member

    Messages:
    3,197
    Likes Received:
    818
    Best Answers:
    7
    Trophy Points:
    320
    #4
    Similar recent situation with me. Was getting 404 errors on hundreds of valid links caused by an error on the target pages. Once I fixed the errors on the target pages, the 404s stopped. What a pain to isolate!
     
    mmerlinn, Dec 19, 2015 IP
  5. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #5
    Which meant you had an object, not an array.

    Depends on which browsers you use for debugging. I've been using classic Opera (and not ChrOpera), FF AND Edge -- as they can each report DIFFERENT errors on different lines in the situation you describe... laughably the error could be on an entirely different line altogether and off by as much as 8 lines in FF depending on the type of JS you're debugging.

    While we're a few months out from your problem, one little observation:
    for(var j = 0; j < statustypes.length; j++){
    	for(var i = 0; i < data.length; i++) {
    		var obj = data[i];
    		if (obj.Individual.status == statustypes[j]){
    			addMemberToList(obj);
    		}
    	}
    }
    Code (markup):
    That's grossly inefficient. You're declaring the var in the innermost loop, and have no short-circuit eval on your testing meaning you are testing multiple states for no reason. First, flip the nesting order of the loops around, then add a break to your match condition.

    for(var i = 0; i < data.length; i++) {
    	var obj = data[i];
    	for (var j = 0; j < statustypes.length; j++) {
    		if (obj.Individual.status == statustypes[j]) {
    			addMemberToList(obj);
    			break;
    		}
    	}
    }
    Code (markup):
    Should give the same result in a fraction the time. HONESTLY though, since you seem to be checking for single characters you may be better off leveraging indexOf instead of a loop, and losing the variable for nothing altogether. Thanks to spiderMonkey being a bit of a re-re depending on the number of data elements, you may be better off adding a length var to the for declaration. (though that's slower in V8, V8 is so much faster than Spidermonkey who cares!).

    Knowing when and when not to storte the length in the loop declaration is a bit like writing assembly where the 8088/8086 is the minimum target; it might be less efficient on 80186/higher systems, but those systems are so much faster elsewhere it doesn't matter on those. If your data set is less than ~24 elements I wouldn't bother. If more, give FF what it wants and plow the rest.

    
    var statustypes = 'mgx';
    
    for(var i = 0, iLen = data.length; i < iLen; i++)
    	if (statusTypes.indexOf(data[i].Individual.status)) != -1)
    		addMemberToList(data[i]);}
    Code (markup):
    Should also be functionally identical. Never brute force loop if you can leverage a language construct to do the same job. The array lookups might be slow, which is why conventional wisdom holds if you use it more than once make a copy to a local var, BUT in this case you only use it twice when there's a match, so whether or not to make stack thrash with the extra variable would hinge on the number of matches to non-matching in the array you are filtering.
     
    deathshadow, Dec 20, 2015 IP
    sarahk likes this.
  6. sarahk

    sarahk iTamer Staff

    Messages:
    28,788
    Likes Received:
    4,528
    Best Answers:
    123
    Trophy Points:
    665
    #6
    I'll go have a look at those and run some learning/test scripts.

    So, is Edge good? I've avoided it so far thinking it'll just turn out to be a whole heap more css hacks that are needed and I don't do design!
     
    sarahk, Dec 20, 2015 IP
  7. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #7
    It's on par with 11 on emulation of older browsers, which was pretty good to begin with compared to 10/earlier. While it's still a bit of a dog in terms of user interface (I say the same thing about Chrome so...) and still trails in key JS and CSS bits, the inspect/developer side of things is pretty good and much like Opera classic will list not only the correct line an error occured on in the scripting, it will also back trace it -- "error on line 200 in function foo called from line 312 in function bar"... the ability to set breakpoints in markup and scripts "out of the box" rivals the tools in other browsers, better in some ways as I think the UI is better thought out -- particularly on multiple displays or really large displays where you have the room to move it to it's own window.

    But really I find you need "one of each engine" to truly get proper error reporting on JS. Pain in the ass really as NOBODY has quite gotten that part right. Sometimes FF's error will be vague but Opera 12's says it more clearly... sometimes Opera 12 just flat out gives up and reports "unexpected error" -- no line numbers or anything... Sometimes IE catches errors the other two don't even see that explains strange behaviors... and sometimes FF will find errors the others give up on...

    About the only odd man out is webkit/blink browsers, their UI is horrible, the scripting error reporting is a joke that doesn't even follow the ECMA guidelines -- but they have one of the best in-built mobile emulations so... You need that one too.
     
    deathshadow, Dec 20, 2015 IP
    sarahk likes this.