JavaScript Library - What's Gone Wrong?

[RobberBaron]

I'm trying to make a simple JavaScript library for my website, using only the things I need rather than loading some big external one. I tried some sample code to make my library a function class in window called 'robberbaron' (I added comments for your benefit):

window.robberbaron=function(a,b){ //create object in windows	var c=this,d=window,e=d.document; //assign a variable to this so different levels can access it	c.rf=e.getElementById; //simple getElementById as c.rf()	c.rt=e.getElementsByTagName; //blah	c.i=c.i=a.indexOf!=null?a.indexOf:function(){return -1} //c.i() will return arguments[0].indexOf or a function returning -1. This is so that it will not error if no argument 0 was assigned.	c.e=!c.w&&c.i('<')>-1&&c.i('>')>-1; //if in a there is a '<' and a '>', set property e to true	c.w=!c.e&&c.i(' ')>-1; //if there is a whitespace character	c.s=e.write; //quick print() function c.s()	c.e&&c.sleep=c.rt(a); //c.sleep is an element called at creation of the robberbaron object, to directly access the requested element. This means if c.e is true, c.sleep is group of elements matching the tag name	!c.e&&!c.w&&c.sleep=c.rf(a); //but if there is no whitespace character and it doesn't have < or >, c.sleep is a single element from the id	return c; //return self for re-use};window.$=window.$_ROBBERBARON=function(a,b){return new window.robberbaron(a,b)} //$ and $_ROBBERBARON are quick functions for creating a new window.robberbaron object.

But then in the same page, I had some test code to see if it was working:

$().s($('fe').sleep[0]);

This should equivelate as document.write(document.getElementById('fe')[0]), so the first thing in 'fe' (fe is the body element, and so the first item is id="fe"). It should write "fe", but instead Google Chrome just says "There is one error: Could not get indexOf of undefined" or something. I thought I'd already fixed that...

[ApocalypeX]

.getElementById(id) returns this, rather than an array like .getElementsByTagName(tag) does.

[RobberBaron]

Yes, however either will have an index of 0, because it would be the first element OR the first value of the element. However, I will try adding tostring() just to see if it produces a result.

[ApocalypeX]

Yes, however either will have an index of 0, because it would be the first element OR the first value of the element. However, I will try adding tostring() just to see if it produces a result.

No, this is an object, not a property. Try it document.getElementById('ele')[0].innerHTML it wont work.

[RobberBaron]

Thanks, I see what you mean. But I found out I could still find the exact error details from the console, and this seems to be the problem:

Uncaught TypeError: Cannot read property 'indexOf' of undefined

This was on line six, which says:

c.i=(a.indexOf!=null?a.indexOf:function(){return -1});

This is supposed to fix that. This means that c.i() is either argument[0].indexOf or a function returning -1 (for obvious reasons).

[ShadowMage]

Try testing for undefined instead of null. If an argument is not passed it will have an undefined value, not a null value.c.i=(a.indexOf!==undefined?a.indexOf:function(){return -1});Also notice I changed your comparison operator from a loose (!=) to a strict (!==) one. Any time you compare to values like null, true, false, or undefined you should use a strict comparison.

[ApocalypeX]

Try testing for undefined instead of null. If an argument is not passed it will have an undefined value, not a null value.c.i=(a.indexOf!==undefined?a.indexOf:function(){return -1});Also notice I changed your comparison operator from a loose (!=) to a strict (!==) one. Any time you compare to values like null, true, false, or undefined you should use a strict comparison.

If that doesn't work try c.i=(typeof a.indexOf !== 'undefined'?a.indexOf:function(){return -1});

[RobberBaron]

Thanks for the tips, however I tried both solutions and it gives the exact same error. It errors on each of the following lines (but these are from where the function was called, not separate problems):

6: c.i=(a.indexOf!==undefined?a.indexOf:function(){return -1});15: window.$=window.$_ROBBERBARON=function(a,b){return new window.robberbaron(a,b)}20: $().s($('fe').sleep[0]);

I tried it twice for both solutions, exactly the same error. In case it helps, I'm using Google Chrome.

[justsomeguy]

Be careful when using the typeof operator, it usually needs parens to do what you're expecting:c.i=((typeof a.indexOf) !== 'undefined' ? a.indexOf : function(){return -1});

[RobberBaron]

Yay fix: c.i=((typeof(a))!=='undefined'?a.indexOf:function(){return -1});But this line is causing problems: c.e&&c.sleep=c.rt(a.replace('<','').replace('>',''));This means, if a has the characters < and >, return an array of elements associated with a minus the < >sUncaught ReferenceError: Invalid left-hand side in assignmentEDITAfter some editing, here is the current script:

window.robberbaron=function(a,b){	var c=this,d=window,e=d.document;	c.rf=e.getElementById;	c.rt=e.getElementsByTagName;	c.i=((typeof(a))!=='undefined'?a.indexOf:function(){return -1});	c.e=!c.w&&c.i('<')>-1&&c.i('>')>-1;	c.w=(!c.e&&c.i(' ')>-1);	c.s=e.write;	c.sleep=(c.e?c.rt(a.replace('<','').replace('>','')):(c.w?null:c.rf(a)));	//!c.e&&!c.w&&c.sleep=c.rf(a);	c.w&&c.s(a);	return c;};window.$=window.$_ROBBERBARON=function(a,b){return new window.robberbaron(a,b)}

And the error:Uncaught TypeError: Illegal invocationThat's line ten: c.sleep=(c.e?c.rt(a.replace('<','').replace('>','')):(c.w?null:c.rf(a)));

[justsomeguy]

I think you should seriously consider using meaningful variable names and expanding your code so it's more readable. It's fine to be concise, but it doesn't do any good if being concise doesn't work. Take these lines, for example:

	c.e=!c.w&&c.i('<')>-1&&c.i('>')>-1;	c.w=(!c.e&&c.i(' ')>-1);

I have literally no idea what c.e, c.w, and c.i represent, but I can see that you're trying to use c.w before you've defined it. If you're going to be doing everything you can with single-line ternary statements, you're going to need to keep this page bookmarked:https://developer.mozilla.org/en/JavaScript...ator_PrecedenceJavascript operators do not go simply left-to-right, they have both precedence and associativity. Some operators go left-to-right, some go right-to-left, each has its own precedence, and if you're going to use ternary statements to assign everything you'll need to use parentheses to make sure the computer is doing what you're expecting it to.You also seem to use a lot of boolean and operators in place of if statements, that might not be the best idea either.

[jeffman]

As far as I can tell, you want this thing to return an element or an object collection. If the value passed in a is not undefined, then we parse a to see if it's a string that starts and ends with angle brackets. If it is, return document.getElementsByTagName(contents of a). If it's not, assume the string is an id, so return document.document.getElementById(a);If I'm on track here, a lot of this could be simplified by 1-2 regular expressions, I think.I'm not sure what you'd use document.write for, unless it's for debugging, maybe.

[RobberBaron]

I never really understood the RegExp object, but I think I'll have a look.Here is my logic:If it is a single word, with no whitespace, assume it is an element id. If it has HTML delimeters, get a collection of matching elements. Else, if there is whitespace, echo the string.So:$('myelement') returns document.getElementById('myelement')$('<myelement>') returns document.getElementsByTagNames('myelement')$('my element') returns document.write('my element')I re-wrote some parts of it:

window.robberbaron=function(){	var a=arguments,b=a[0],c=a[1],d=this,e=window,f=e.document;	d.rf=f.getElementById;	d.rt=f.getElementsByTagName;	d.s=f.write;	if (b!==undefined){		d.i=b.indexOf;		d.w=(d.i(' ')>-1);		d.t=(d.i('<')>-1)&&(d.i('>'));		if(d.w){d.s(b)};		(d.t&&!d.w)&&d.sleep=d.rt(b.replace('<','').replace('>',''));		if(!d.w&&!d.t){d.sleep={0:d.rf(b)}};	};};window.$=window.$_ROBBERBARON=function(){return new window.robberbaron(arguments[0],arguments[1])};

The only problem area is this line (Uncaught TypeError: Illegal invocation):if(d.w){d.s(};All this means is, if (b.indexOf(' ')>-1) { document.write( }, or "if there is a whitespace character, echo the full string".I've re-ordered it loads of times, yet it still remains the same error. In fact it was originally just (d.w)&&d.s(:)If I comment out that line, I get an error because of the test command, cannot read property 0 of undefined:$().s($('fe').sleep[0]['id']);Yet I checked the conditions and it should be working. But it gives me the type error again even if I just use $().s() at all.Sorry if I haven't been thorough or attentive, I'm sleepy.

[jeffman]

Oops. See the next post.

[jeffman]

There are a few flaws in your assignment logic:

d.i=b.indexOf;

Tell me if I'm wrong, but here's what I think you think: that executing d.i('sometext') will return the index of 'sometext' in b.It will not. The statement above assigns only the indexOf method to d.i . It does not somehow drag along the value of b. Executing d.i will not look for 'sometext' in b. It will look for 'sometext' in d. Since the string value of an object is "[object]" in some browsers and "[object Object]" in others, the function will in fact execute, but the return value will be based on one of those strings, not the content of b.So right away, the value of d.w is useless. The same problem occurs here:

d.s=f.write

I believe you are assuming that executing d.s will execute document.write. It will not. The statement has only assigned the write method to d.s . The document object has not been carried along with it. In this case, there is no meaningful relationship between your d object and the write method. So d cannot execute write at all. That is the meaning of the error. d cannot invoke a method that does not belong to the type of object it is.These problems are pretty fundamental to the assumptions you have made while developing this object.There are ways around these problems if you want to pursue the same train of thought. Let me suggest that you test assignments like these individually before you write additional code that depends on their success.

[jeffman]

It's hard to write efficient code without eventually running into regular expressions. This code snippet might get you thinking.

var s="<element>";var m;var re = /<(.+)>/;if (m = s.match(re) ) {   alert(m[1]);}

Obviously this can all be condensed.

[RobberBaron]

Thanks for all the help. I understand now, assigning a variable makes a clone, rather than linking to the actual object.The RegExp code sample you provided is a much more efficient way of teaching about how it works than anything else. It's also very useful that at index 1 of the match result you are given the text without the delimeters "<" and ">".Once I fixed lines 3 and 4 it worked perfectly, no errors! The current script is:

window.robberbaron=function(){	var a=arguments,b=a[0],c=a[1],d=this,e=window,f=e.document;	d.rf=function(g){return f.getElementById(g)};	d.rt=function(g){return f.getElementsByTagName(g)};	d.s=function(g){return f.write(g)};	if (b!==undefined){		d.i=function(g){return b.indexOf(g)};		d.w=(d.i(' ')>-1);		d.t=b.match(/<(.+)>/);		d.w&&d.s(b);		if(d.t&&!d.w){d.sleep=d.rt(d.t[1])};		if(!d.w&&!d.t){d.sleep=[d.rf(b)]};	};};window.$=window.$_ROBBERBARON=function(){return new window.robberbaron(arguments[0],arguments[1])};

And the test script was:

$().s($('<body>').sleep[0]['id']);$('<br />');$().s($('fe').sleep[0]);$('<br />');$('Here are some words!');

Which outputs:fe[object HTMLBodyElement]Here are some words!Thanks guys!