Help for a Beginner

Discussion in 'JavaScript' started by Soriano, Nov 6, 2008.

  1. #1
    Hello,

    I am new to javascript and was hoping someone could help me. I am taking a night class to teach myself how to make websites to maybe start a second business on the side. The instructor gave us some problems that we may want to try. He said while it would be hard to do them with our current tools that it was possible.

    Here is one problem I have been working on:

    # Let the user type a word or sentence. Count the number of times each letter was used. Display the answer in color. For example, if the user types: "this is the end", you would display:
    1 d
    2 e
    2 h
    2 i
    1 n
    2 t

    We are only up to For loops so we aren't allowed to use arrays (he said this would make us respect arrays later )

    Here is what I have come up with so far:

    <html>
    <head>
    <script type="text/javascript">
    
       var a=0;
       var b=0;
       var c=0;
       var d=0;
       var e=0;
       var f=0;
       var g=0;
       var h=0;
       var i=0;
       var j=0;
       var k=0;
       var l=0;
       var m=0;
       var n=0;
       var o=0;
       var p=0;
       var q=0;
       var r=0;
       var s=0;
       var t=0;
       var u=0;
       var v=0;
       var w=0;
       var x=0;
       var y=0;
       var z=0;
       var userstring=0;
       
    function sentance_prompt()
    {
    var userstring=prompt("Please enter any sentance that contains letters only (No Punctuation Please or Capital Letters!)");
    if (userstring!="" && userstring!=null)
       {
       var len=userstring.length;
       document.write("Your sentance was: " + userstring + ". <br>");
       document.write("Your sentance has " + len + " characters in it <br>");
       document.write("The letter frequency in your sentances is: <br>");
       
       for(inc=0 ; inc <= len ; inc++)
       {
       letter=userstring.substring(inc,inc++);
       if (letter==a)
          {
          a=a++;
          }
       else if (letter==b)
          {
          b=b++;
          }
       else if (letter==c)
          {
          c=c++;
          }
       else if (letter==d)
          {
          d=d++;
          }
       else if (letter==e)
          {
          e=e++;
          }
       else if (letter==f)
          {
          f=f++;
          }
       else if (letter==g)
          {
          g=g++;
          }
       else if (letter==h)
          {
          h=h++;
          }
       else if (letter==i)
          {
          i=i++;
          }      
       else if (letter==j)
          {
          j=j++;
          }
       else if (letter==k)
          {
          k=k++;
          }
       else if (letter==l)
          {
          l=l++;
          }
       else if (letter==m)
          {
          m=m++;
          }      
       else if (letter==n)
          {
          n=n++;
          }
       else if (letter==o)
          {
          o=o++;
          }      
       else if (letter==p)
          {
          p=p++;
          }
       else if (letter==q)
          {
          q=q++;
          }      
       else if (letter==r)
          {
          r=r++;
          }
       else if (letter==s)
          {
          s=s++;
          }
       else if (letter==t)
          {
          t=t++;
          }
       else if (letter==u)
          {
          u=u++;
          }      
       else if (letter==v)
          {
          v=v++;
          }
       else if (letter==w)
          {
          w=w++;
          }                                     
       else if (letter==x)
          {
          x=x++;
          }
       else if (letter==y)
          {
          y=y++;
          }        
       else (letter==z)
          {
          z=z++;
    	  }
       }
       document.write(a+" a <br>");
       document.write(b+" b <br>");
       document.write(c+" c <br>");
       document.write(d+" d <br>");
       document.write(e+" e <br>");
       document.write(f+" f <br>");
       document.write(g+" g <br>");
       document.write(h+" h <br>");
       document.write(i+" i <br>");
       document.write(j+" j <br>");
       document.write(k+" k <br>");
       document.write(l+" l <br>");
       document.write(m+" m <br>");
       document.write(n+" n <br>");
       document.write(o+" o <br>");
       document.write(p+" p <br>");
       document.write(q+" q <br>");
       document.write(r+" r <br>");
       document.write(s+" s <br>");
       document.write(t+" t <br>");
       document.write(u+" u <br>");
       document.write(v+" v <br>");
       document.write(w+" w <br>");
       document.write(x+" x <br>");
       document.write(y+" y <br>");
       document.write(z+" z");  
       
       }
    }
    
    
    
    
    
    
       
     
    
    </script>
    </head>
    <body>
    
    <input type="button" onclick="sentance_prompt()" value="Display a prompt box" />
    
    </body>
    </html>
    Code (markup):
    For some reason it gets up to the loop and doesn't work after that. I have been beating my head against the screen trying to figure out why but cannot see what is wrong. Can someone see what I'm missing or what stupid mistake I've made.

    Thanks in advance!

    -MS
     
    Soriano, Nov 6, 2008 IP
  2. LogicFlux

    LogicFlux Peon

    Messages:
    2,925
    Likes Received:
    102
    Best Answers:
    0
    Trophy Points:
    0
    #2
    letter=userstring.substring(inc,inc++);

    The problem with this line is that when you pass inc++ as the "stop" parameter for substring() you are passing in the same value as inc, and then after the value is passed, 1 is added to it, which screws up the loop count.


    Use this: userstring.substring(inc,(inc+1));



    if (letter==a)

    Strings need to be surrounded by double or single quotes. That's ' or ". Otherwise you're asking if the variables/object letter equal the variable/object a.

    Use this:

    if (letter=='a')


    a=a++;

    This first assigns the value of a to a and then adds 1 to the value on the right side of the assignment operator(=). You're using what is called the post-increment operator. You have two choices, you could use the pre-increment operator and do this:

    a=++a;

    or either of these by themselves on the line:

    ++a;

    or

    a++;

    Context:

    if (letter=='a')
    {
    ++a;
    }
     
    LogicFlux, Nov 7, 2008 IP
  3. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #3
    Gah, and people wonder why I consider most computer science degrees to not be worth as much as a sheet of toilet paper - let's teach the completely wrong way of doing things first... Brilliant. As you obviously figured out, an array is the proper way to handle this - but you've got other things biting you too. The lack of consistant indentation and use of 'new AT&T' style opening and closing brackets makes it unclear if everything is even opened or closed properly. The overuse of double quotes is probably making the code take 20-30% longer to execute, (has the difference between singles and doubles even been covered yet? It's not just 'personal preference' as some educators would have you think), that big string of if statements are all using the same variable, so a SWITCH would be more efficient, and you've got MAJOR logic flow issues.

    See this:
    	for(inc=0 ; inc <= len ; inc++) {
    		letter=userstring.substring(inc,inc++);
    Code (markup):
    The second parameter on substring is LENGTH. You only want one character. inc++ inherently INCREASES the your inc variable... because of this on the first pass this is what's run:

    inc=0;
    letter=userstring.substring(0,1); inc++ so inc now == 1

    on the next pass you are sending it:
    inc++; // inc now == 2
    letter=userstring.substring(2,3); inc ++ so now inc == 3

    on the pass after that the for loop will change inc to 4, while your substring will be passed 4,5 and change inc to 5.

    You see the problem? By calling inc++ on each line, you are changing the value of inc when you don't want to. Since you only want to test for one letter, the second value should ALWAYS be one, because the second parameter for substring is LENGTH. Start,length - NOT start, end... Though you shouldn't even be using substr in the first place since there's a function called 'charat' that pulls just one character.

    Logicflux hit it on the head on your if statements needing quotes, and was also close (he missed the big flaw) on the if statement. The a=a++ is a redundant assignment. a++ inherently adds one to a - I beleive all you wanted to do there is a++; (++a is cute, but not really important to change the precedence)

    I would also suggest moving the whole thing to lower case.... and your string is not going to jail, so that's sentence, not sentance (ok, nitpick) - and a simple final else with an 'other' variable would let you trap punctuation and other characters.

    ... and inputs outside a form are bad markup, so I'd use an anchor - even if it is just a test, it's best to NOT form the bad habits now.

    If I wrote that using separate variables, it would probably end up this:
    <html><head>
    
    <title>String letter counts</title>
    
    <script type='text/javascript'><!--
    
    var 
    	a=0,b=0,c=0,d=0,e=0,f=0,g=0,h=0,i=0,
    	j=0,k=0,l=0,m=0,n=0,o=0,p=0,q=0,r=0,
    	s=0,t=0,u=0,v=0,w=0,x=0,y=0,z=0,other=0;
    
    function sentence_prompt() {
    	var userString=prompt('Please enter any sentence');
    	
    	if (userString!='' && userString!=null)	{
    		var len=userString.length;
    		document.write(
    			'Your sentence was: '+userString+'<br>'+
    			'Your sentence has '+len+' characters in it<br>'+
    			'The letter frequency in your sentences is:<br>'
    		);
    		
    		userString=userString.toLowerCase();
    		
    		for (inc=0 ; inc <= len ; inc++) {
    			switch (userString.charAt(inc)) {
    				case 'a': a++; break;
    				case 'b': b++; break;
    				case 'c': c++; break;
    				case 'd': d++; break;
    				case 'e': e++; break;
    				case 'f': f++; break;
    				case 'g': g++; break;
    				case 'h': h++; break;
    				case 'i': i++; break;
    				case 'j': j++; break;
    				case 'k': k++; break;
    				case 'l': l++; break;
    				case 'm': m++; break;
    				case 'n': n++; break;
    				case 'o': o++; break;
    				case 'p': p++; break;
    				case 'q': w++; break;
    				case 'r': r++; break;
    				case 's': s++; break;
    				case 't': t++; break;
    				case 'u': u++; break;
    				case 'v': v++; break;
    				case 'w': w++; break;
    				case 'x': x++; break;
    				case 'y': y++; break;
    				case 'z': z++; break;
    				default: other++; break;
    			}
    		}
    		
    		document.write(
    			a+' a<br>'+b+' b<br>'+c+' c<br>'+d+' d<br>'+e+' e<br>'+f+' f<br>'+
    			g+' g<br>'+h+' h<br>'+i+' i<br>'+j+' j<br>'+k+' k<br>'+l+' l<br>'+
    			m+' m<br>'+n+' n<br>'+o+' o<br>'+p+' p<br>'+q+' q<br>'+r+' r<br>'+
    			s+' s<br>'+t+' t<br>'+u+' u<br>'+v+' v<br>'+w+' w<br>'+x+' x<br>'+
    			y+' y<br>'+z+' z<br>'+other+' other<br>'
    		);
    	} else {
    		alert('You entered no string');
    	}
    }
    
    --></script>
    
    </head><body>
    
    <a href="#" onclick="sentence_prompt(); return false;">Display a prompt box</a>
    
    </body></html>
    Code (markup):
    Though if you want to see why an array would be so much better...

    <html><head>
    
    <title>String letter counts</title>
    
    <script type='text/javascript'><!--
    
    
    function sentence_prompt() {
    	var letterCount=new Array();
    	var userString=prompt('Please enter any sentence');
    	
    	if (userString!='' && userString!=null)	{
    		var len=userString.length;
    		document.write(
    			'Your sentence was: '+userString+'<br>'+
    			'Your sentence has '+len+' characters in it<br>'+
    			'The letter frequency in your sentences is:<br>'
    		);
    		
    		for (t=30; t<128; t++) { letterCount[t]=0; }
    		
    		userString=userString.toLowerCase();
    		
    		for (t=0; t<len; t++) {
    			b=userString.charCodeAt(t);
    			if (b<32) {
    				letterCount[30]++;
    			} else if (b<128) {
    				letterCount[b]++;	
    			} else letterCount[31]++;
    		}
    		
    		for (t=33; t<128; t++) {
    			if (letterCount[t]>0) {
    				document.write(letterCount[t]+' '+String.fromCharCode(t)+'<br>');
    			}
    		}
    		if (letterCount[32]>0) {
    			document.write(letterCount[32]+' spaces<br>');
    		}
    		if (letterCount[30]>0) {
    			document.write(letterCount[30]+' non printing<br>');
    		}
    		if (letterCount[31]>0) {
    			document.write(letterCount[31]+' non 7 bit ASCII<br>');
    		}
    		
    	} else {
    		alert('You entered no string');
    	}
    }
    
    --></script>
    
    </head><body>
    
    <a href="#" onclick="sentence_prompt(); return false;">Display a prompt box</a>
    
    </body></html>
    Code (markup):
    I use index 30 of the array for ascii codes 0..31, and index 31 for codes 128..~, this lets us only have to initialize values 30..128 in our array. Instead of testing what we are outputting inside the output loop, we just go through the values we want to display characters for, then manually check the three exceptions after - moving the if's outside the loop speeds execution. This is all possible because charCodeAt returns the numeric character code of the letter in ASCII (whereas charAt returns the character as a string).

    Hope this helps.
     
    deathshadow, Nov 7, 2008 IP
  4. LogicFlux

    LogicFlux Peon

    Messages:
    2,925
    Likes Received:
    102
    Best Answers:
    0
    Trophy Points:
    0
    #4
    This is wrong. The second parameter isn't length, it's the character index of the string to stop at.

    http://www.bloggingdeveloper.com/post/JavaScript-substring-Function.aspx
     
    LogicFlux, Nov 8, 2008 IP
    bogart likes this.
  5. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #5
    What the??? My bad, I forgot that javascript has two almost identical functions - substr and substring... since most every other language only has the former.

    Logic still doesn't work though... should be inc+1 and not inc++ since you do not want to change the VALUE of inc... NOT that it is even the right function in the first place to extract a single character.

    substring(inc,inc+1)
    substr(inc,1)
    charAt(inc)

    All do the same thing, in descending order of execution time. The first one is two variables to pass, an operator on one parameter, and math inside the function to set the number of iterations. The second is also two variables passed, though the number of iterations is already calculated but must still be checked via a loop. The last one passes one variable and can hard-code pulling just one character - making it several magnitudes of order faster.

    substring(inc,inc++) is therin the same, but changes the value of inc which is undesired... unless of course you swapped the FOR for a WHILE. It would skip every other character, and might return NO characters depending on the precedence of the ++ over the first parameter. Since the addition should occur before the parameters are passed to the function, it should increment the inc varable, then pass the modified value to BOTH parameters... Though being .js is a sloppy interpreted language it probably stacks them instead of pointering them.

    var a=2;
    var tst='test';
    var b=tst.substring(a,a++);
    document.write(a);

    will return 3 as the value of A. Put that inside a for loop for A, and it will skip every other letter... which is what you said.

    Uhg, I'm switching languages WAY too often these days. Could be worse, at least it's not Clipper.
     
    deathshadow, Nov 8, 2008 IP