By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
459,317 Members | 1,290 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 459,317 IT Pros & Developers. It's quick & easy.

help needed with function

P: 5
Hi guys,

I'm trying to complete a function that does the following:

/* Tests if all balls selected match all balls drawn in the same order

The function takes two arguments:
an array of drawn balls
an array of selected balls.

The function code compares each ball in one array with the ball at the same position in the other array.

The function returns:
true if the arrays contain the same numbers in the same order
false otherwise.
*/

The code I have written is

Expand|Select|Wrap|Line Numbers
  1.  
  2. {
  3. for (var index = 0; index < drawnBalls.length; index = index + 1)
  4.     {
  5.         if(drawnBalls[index] == selectedBalls[index])
  6.         {
  7.         return true;
  8.         }
  9.         else
  10.         {
  11.         return false;
  12.         }
  13.     }
  14.  
  15. }
  16.  
I have two arrays drawnBalls and selectedBalls, which will both be the same length and need to check if the numbers in the arrays match in the sameorder.
Instead of returning an overall return of true or false if all the balls match, it is looping, giving me a separate result for each element. Can anyone give me some guidance on how to achieve my overall objective of, simply returning true if the values in the array are the same and in the same order, and false otherwise. Thanks in advance for any help.
May 22 '09 #1
Share this Question
Share on Google+
17 Replies


P: 3
The best thing to due is use a variable in the loop. Then after the loop is done then return the variable. So the logic flows like this. First set the variable to false. Then loop through array setting variable to true if you have a match. If all elements in drawnBalls equall all elements in selectBalls. Then the variable will remain true. if any element doesn't match then set to false and break out of the for setting index to last value. This is due to the fact that if one element drawnBalls is not equal to selectBalls then the arrays are the same and you return false. Something like the following.

Expand|Select|Wrap|Line Numbers
  1. var match=new Boolean();
  2. for (var index = 0; index < drawnBalls.length; index = index + 1)
  3.     {
  4.         if(drawnBalls[index] == selectedBalls[index])
  5.         {
  6.         match=new Boolean(true);
  7.         }
  8.         else
  9.         {
  10.         match=new Boolean (false);
  11.                                 index=drawnBalls.length;
  12.         }
  13.     }
  14.  
  15. }
  16. return match.toString();
May 23 '09 #2

acoder
Expert Mod 15k+
P: 16,027
Although the idea is correct, the code could definitely be optimised. There's no need for a variable. If there is a non-match, just return false. If the loop ends and you're still in the function, that means that all values match, so return true at that point.
May 23 '09 #3

P: 5
Thanks for the reply both,

I'm trying to amend my code to take into account both suggestions and have come up with:

Expand|Select|Wrap|Line Numbers
  1. var drawnBalls = [1,2,3,4,5];
  2. var selectedBalls = [1,2,3,4,5];
  3.  
  4. document.write(drawnBalls + '<BR>');
  5. document.write(selectedBalls + '<BR>');
  6.  
  7. function allBallsMatch(drawnBalls, selectedBalls)
  8.  
  9.     {
  10.     for (var index = 0; index < drawnBalls.length; index = index + 1)
  11.         {
  12.             if(drawnBalls [index] == selectedBalls[index])
  13.             {
  14.             return true;
  15.             }
  16.             else
  17.             {
  18.             return false;
  19.  
  20.                 index=drawnBalls.length; 
  21.             }
  22.  
  23.         }
  24.     return true;
  25.     }    
This still doesn't seem to be giving me the required outcome, can you please point out where I'm going wrong?
May 24 '09 #4

100+
P: 270
Hello boobikins,
It seems that you are returning true/false every time you compare the elements of the array thats why its not working.. instead you can use a flag variable to point out that all elements in array match or not.


Expand|Select|Wrap|Line Numbers
  1. /*
  2. *                   ||  JAI BHAVANI ||
  3. */
  4. function allBallsMatch(drawnBalls, selectedBalls){
  5.     var flag=true;
  6.     for (var index = 0; index < drawnBalls.length; index = index + 1){
  7.             if(drawnBalls [index] == selectedBalls[index]){
  8.                 flag=true;
  9.             }
  10.             else{
  11.                 flag=false;
  12.             }
  13.  
  14.     }
  15.     return flag;
  16. }
i hope that, it works for you.

kind regards,
Nitin Sawant
May 24 '09 #5

acoder
Expert Mod 15k+
P: 16,027
Nitin, your code wouldn't work because, say, if it doesn't match the first one and matches the rest or even the last items, then it would return true.
May 25 '09 #6

P: 5
I have never used flag, so how can I get my function to work returning an overall true or false? Still stuggling...
May 25 '09 #7

gits
Expert Mod 5K+
P: 5,390
a flag is just a variable that holds a value for some easy comparison ... there are several ways to achieve your goal and a flag in your case would better hold a number ... acoder gave you a very good hint already ... so i'll repeat it and perhaps give you one more idea :) ... so let me write a working example in pseudocode:

Expand|Select|Wrap|Line Numbers
  1. var drawnBalls    = [1,2,3,4,5];
  2. var selectedBalls = [1,2,3,4,6];
  3.  
  4. function allBallsMatch(drawnBalls, selectedBalls) {
  5.     var dbl = drawnBalls.length;
  6.     var sbl = selectedBalls.length;
  7.  
  8.     var compArray = [];
  9.  
  10.     // or use a 'flag'
  11.     // var flag = 0;
  12.  
  13.     if (dbl == sbl) {
  14.         for (var i = 0; i < dbl; ++i) {
  15.             var b = drawnBalls[i];
  16.  
  17.             // in case b matches the number in selectedBalls
  18.             // push() something in the compArray or add 1 to 
  19.             // your flag
  20.             // else break; the loop 
  21.         }         
  22.     }
  23.  
  24.     return compArray.length == dbl;
  25.  
  26.     // or in case you have used the flag
  27.     // return flag == dbl;
  28. }
  29.  
kind regards
May 25 '09 #8

omerbutt
100+
P: 638
@gits
would it do the work as far i understood you want that all the indexes in both the arrays should have same values/ balls wotever else it should return false
see if this helps
Expand|Select|Wrap|Line Numbers
  1.  function allBallsMatch(drawnBalls, selectedBalls)  
  2.      {
  3. var flg=0;
  4.      for (var index = 0; index < drawnBalls.length; index = index + 1)
  5.          {
  6.              if(drawnBalls [index] != selectedBalls[index])
  7.              {
  8.                   flg=1;
  9.              }
  10.  
  11.          }
  12.          if(flg==0){
  13.            return true;
  14.           }else{
  15.            return false;
  16.          }
  17.      }
  18.  
May 25 '09 #9

gits
Expert Mod 5K+
P: 5,390
have a look at post #6 ... the shown code has the same problem.

kind regards
May 25 '09 #10

acoder
Expert Mod 15k+
P: 16,027
On top of that, there's no need for any flags. You can just return false when there's definitely a non-match.
May 25 '09 #11

omerbutt
100+
P: 638
@gits
gits if this was for me then i dont think that i am returning true at any place iit would return false only if even any one of them would not match, but may be there is something wrong with it.
May 26 '09 #12

gits
Expert Mod 5K+
P: 5,390
yes ... i overlooked the braces ... so it would work ... it's just bad readable ... and as acoder said it would work but could be optimized to something like this (to avoid needless memory-usage and operations):

Expand|Select|Wrap|Line Numbers
  1. function allBallsMatch( drawnBalls, selectedBalls ) {
  2.     for ( var i = 0, l = drawnBalls.length; i < l; ++i ) {
  3.         if ( drawnBalls[i] != selectedBalls[i] ) {
  4.             return false;
  5.         }
  6.     }
  7.     return true;
  8. }
May 26 '09 #13

100+
P: 270
@acoder
Hello acoder,
thanks, now i got the mistake
May 28 '09 #14

omerbutt
100+
P: 638
@NitinSawant
suppose your very forst value in the array does not match and the rest all do it would set flag false for the first and then for all the rest values it would keep on setting the flag true :) isnt it , seee carefully
here is an example arrays with the elements try comparing these
Expand|Select|Wrap|Line Numbers
  1. drawnBalls [0,1,2,3,4,5]
  2. selectedBalls[1,1,2,3,4,5]
  3.  
May 28 '09 #15

100+
P: 270
Expand|Select|Wrap|Line Numbers
  1. <!-- 
  2.         ||    JAI BHAVANI    ||
  3.         Author: Nitin Sawant
  4.         Email: nitinsawant@netbeans.org
  5. -->
  6. <html>
  7. <head>
  8.     <script type="text/javascript">
  9.         var drawnBalls = new Array(1,2,3,4,5);
  10.         var selectedBalls = new Array(1,2,3,4,5);
  11.         function allBallsMatch(drawnBalls, selectedBalls){
  12.             var flag=true;
  13.             for (var index = 0; index < drawnBalls.length;index = index + 1){
  14.                     if(drawnBalls [index] !=selectedBalls[index]){
  15.                         return false;
  16.                     }
  17.  
  18.             }
  19.             return true;
  20.         }
  21.     </script>
  22. </head>
  23. <body>
  24.     <script type="text/javascript"> 
  25.         document.write("Contents of drawnBalls: <BR>");
  26.         for(var temp=0;temp<drawnBalls.length;temp=temp+1){
  27.             document.write(drawnBalls[temp]+ '<BR>');
  28.         }
  29.         document.write('<BR>');
  30.         document.write("Contents of selectedBalls: <BR>");
  31.         for(var temp1=0;temp1<selectedBalls.length;temp1=temp1+1){
  32.             document.write(selectedBalls[temp1]+ '<BR>');
  33.         }
  34.  
  35.         //check if both arrays are equal
  36.         if(allBallsMatch(drawnBalls, selectedBalls)){
  37.             document.write("All balls match <BR>");
  38.         }else{
  39.             document.write("All balls doesn't match <BR>");
  40.         }
  41.     </script>
  42. </body>
  43. </html>
May 28 '09 #16

omerbutt
100+
P: 638
@NitinSawant
:) yeah thats it good to go
regards,
Omer Aslam
May 28 '09 #17

gits
Expert Mod 5K+
P: 5,390
just some hints to optimize that:

Expand|Select|Wrap|Line Numbers
  1. // arrays should be instantiated with the literals
  2. // for better performance -> so write this:
  3. var drawnBalls = new Array(1,2,3,4,5);
  4.  
  5. // as:
  6. var drawnBalls = [ 1, 2, 3, 4, 5 ];
  7.  
  8. function allBallsMatch(drawnBalls, selectedBalls){
  9.     // the variable flag is useless in that code so remove it
  10.     // var flag=true;
  11.  
  12.     // it is not optimal to always evaluate the length of the array
  13.     // so it's better to do it just once and even the incrementation
  14.     // should be written shorter -> so instead of:
  15.     // for ( var index = 0; index < drawnBalls.length; index = index + 1 ) {
  16.     // use:
  17.     for ( var index = 0, l = drawnBalls.length; index < l; ++index ) {
  18.         if ( drawnBalls[index] != selectedBalls[index] ) {
  19.             return false;
  20.         }
  21.     }
  22.     return true;
  23. }
  24.  
kind regards
May 28 '09 #18

Post your reply

Sign in to post your reply or Sign up for a free account.