Page 1 of 2 12 LastLast
Results 1 to 20 of 21
  1. #1
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Factorial method bug

    Hi,

    I've written 2 methods that supposed to return eventually the sum of factorials in an array (every element consists only 1 digit).

    After checking it time after time and going throught it to figure what's wrong, I've realized that after the first digit the method calculates, it's just taking every following digit and returns "1" as it's factorial value.

    I just can't figure out why! I've copied it so u can have a look:

    Java Code:
    public static int sumFactorial (int[] B , int numElementsB) { //sumFactorial method starts
    		int sumFactorial=0;
    		for (int i=1 ; i < numElementsB ; i++ ) {
    			
    			if (B[i] <= 1 ) sumFactorial++;
    			else sumFactorial+=factorial(B[i]);		
    		}//for ends
    		
    		return sumFactorial;
    		
    		
    	}
    	
    	public static int factorial (int num) { //factorial method starts
    		int facNum=1;
    		for (int i = 1 ; i <= num ; i++){
    			facNum=(facNum*i);
    		}//for ends
    		return facNum;
    		
    		
    		
    	}

  2. #2
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,565
    Rep Power
    12

    Default Re: Factorial method bug

    It works for me. Ie the following code prints 9 as expected.

    Java Code:
    public class FacTest {	
    	public static void main(String args[]) {
    		int[] arr = new int[] {0, 1, 2, 3, 2, 1};
    		int num = 4;
    		
    			// should print 9 ie 1!+2!+3!
    		System.out.println(sumFactorial(arr, num));
    	}
    
    		/**
    		 * Returns the sum of the factorials of a given int array from index 1 to
    		 * just before a given index.
    		 *
    		 * @param  B             the array to be summed 
    		 * @param  numElementsB  the index just after where the summation ends
    		 */
    	public static int sumFactorial (int[] B , int numElementsB) {
    		int sumFactorial=0;
    		
    		for (int i=1 ; i < numElementsB ; i++ ) {
    			if (B[i] <= 1 ) sumFactorial++;
    			else sumFactorial+=factorial(B[i]);     
    		}
    		return sumFactorial;
    	}
    
    	public static int factorial (int num) {
    		int facNum = 1;
    		
    		for (int i = 1 ; i <= num ; i++){
    			facNum=(facNum*i);
    		}
    		return facNum;
    	}
    }
    Note the comment on the sumFactorial() method. Perhaps you mean it to do something different?

    In particular if you want numElementsB factorials to be summed (rather than numElementsB-1 of them) you had better change the for loop in sumFactorial().

  3. #3
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    when i try it with the array {3,3}
    it gives me as factorial sum 7 instead of 12

    in the first element it does it correct, on the second, third, etc it just gives "1" instead of calculating it.

    I've tried changing the loop index to be until numElementB but it didn't help :(

  4. #4
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,565
    Rep Power
    12

    Default Re: Factorial method bug

    It's hard to tell why you are getting 7 without seeing the code... As I said the code you posted does exactly what it should: it sums the factorials for a portion of the array it is passed. If you are seeing something else, that's because of other code.

  5. #5
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    hmm.. i see..
    I haven't found anything myself, that's why i posted the thread.
    I'll try again to go over all the code, then if i haven't found the problem I'll probably post more of the code.
    Now at least i know where the problem doesn't exist.

    Tnx again :)

  6. #6
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,565
    Rep Power
    12

    Default Re: Factorial method bug

    In the for loop of the sumFactorial() method try printing out the values of B[i] at the start. Perhaps the array does not contain the values you think, or they are not in the places you think they are. 7=3!+0! so it may be that you are looking at the array values one place "shifted".

  7. #7
    2by4 is offline Banned
    Join Date
    Dec 2011
    Posts
    143
    Rep Power
    0

    Default Re: Factorial method bug

    Quote Originally Posted by Mapisto View Post
    hmm.. i see..
    I haven't found anything myself, that's why i posted the thread.
    I'll try again to go over all the code, then if i haven't found the problem I'll probably post more of the code.
    Now at least i know where the problem doesn't exist.

    Tnx again :)
    Double check that you have saved and compiled the version you have posted.

    If you compile without saving your latest changes, or you save without compiling, it can be very frustrating!!!

  8. #8
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    Oh hell :|

    I believe I've found my problem!

    In my last thread you've told me all that thing with the method comparing it to an orange seller

    Well, if what you say it's true (and of course it is!) then i've changed the array with another method before it got to the factorial method! That's because i had no idea it changed the array in the main :(

    now I'll have to think how to fix this........ unending problems :( I'm sure he thought about all this when he wrote this exercise.......................................... .....................

  9. #9
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    guys, take a look at this method:

    Java Code:
    public static int differentDigits (int[] B , int numElementsB) { //differentDigits method starts
    		
    		int count = 0; 
    		for (int i = 1 ; i < numElementsB ; i++) {
    			if (B[i]>=0) count++;
    			for (int n = (i+1) ; n < numElementsB ; n++){
    					
    					if (B[i]==B[n]) {
    						B[n] =-1;
    					}//2st if ends
    				
    				
    			}//2st for ends
    			
    			
    			
    		}//1st for ends
    		
    		return count;
    This one is supposed (and does) return how many different digits are in a given array.

    Well, u can see from the way I've written it that i changed the array completely without knowing it will affact the array in the main method...

    now I have 2 options:
    option 1: change the method in a way that it does not change the array.
    option 2: find a way to save the same array without the changes b4 invoking the method, and then give it back after invoking it.

  10. #10
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    Now when i think of it, option 2 won't be available.

    There are given lines to the program that I must not change......... all must happen inside the method.


    edit:
    I could just create a new array in the method and be4 it starts changing the original array, copying its values to the new 1.
    in the end, I could just insert all the saved values to the array that has been changed.

    But this way seems barbaric to me :| there must be an other way to do it without creating a new array with 1000 elements that takes so much memory!


    edit:

    I've tried it after all, though it didn't work and i wonder why:

    Java Code:
    public static int differentDigits (int[] B , int numElementsB) { //differentDigits method starts
    		
    		int[] C = new int[1000];
    		int count = 0; 
    		C=B;
    		for (int i = 1 ; i < numElementsB ; i++) {
    			if (B[i]>=0) count++;
    			for (int n = (i+1) ; n < numElementsB ; n++){
    					
    					if (B[i]==B[n]) {
    						B[n] =-1;
    					}//2st if ends
    				
    				
    			}//2st for ends
    			
    			
    			
    		}//1st for ends
    		B=C;
    		return count;
    		
    		
    		
    	}
    Last edited by Mapisto; 12-17-2011 at 02:41 PM.

  11. #11
    2by4 is offline Banned
    Join Date
    Dec 2011
    Posts
    143
    Rep Power
    0

    Default Re: Factorial method bug

    Your original code was nearly there. It doesn't write into B[]. Did you see pbrockway2's comment at the end?

    Quote Originally Posted by pbrockway2 View Post
    It works for me. Ie the following code prints 9 as expected.

    Java Code:
    public class FacTest {	
    	public static void main(String args[]) {
    		int[] arr = new int[] {0, 1, 2, 3, 2, 1};
    		int num = 4;
    		
    			// should print 9 ie 1!+2!+3!
    		System.out.println(sumFactorial(arr, num));
    	}
    
    		/**
    		 * Returns the sum of the factorials of a given int array from index 1 to
    		 * just before a given index.
    		 *
    		 * @param  B             the array to be summed 
    		 * @param  numElementsB  the index just after where the summation ends
    		 */
    	public static int sumFactorial (int[] B , int numElementsB) {
    		int sumFactorial=0;
    		
    		for (int i=1 ; i < numElementsB ; i++ ) {
    			if (B[i] <= 1 ) sumFactorial++;
    			else sumFactorial+=factorial(B[i]);     
    		}
    		return sumFactorial;
    	}
    
    	public static int factorial (int num) {
    		int facNum = 1;
    		
    		for (int i = 1 ; i <= num ; i++){
    			facNum=(facNum*i);
    		}
    		return facNum;
    	}
    }
    Note the comment on the sumFactorial() method. Perhaps you mean it to do something different?

    In particular if you want numElementsB factorials to be summed (rather than numElementsB-1 of them) you had better change the for loop in sumFactorial().

  12. #12
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    yea, I've read it and he's right, but it still doesn't solve the main problem.

    Can't i just copy 2 arrays like i did ? or do i need to refer into each value seperately with a new loop?

  13. #13
    2by4 is offline Banned
    Join Date
    Dec 2011
    Posts
    143
    Rep Power
    0

    Default Re: Factorial method bug

    Do you think you have a problem because of this test? What did you use for numElementsB in this test?

    Quote Originally Posted by Mapisto View Post
    when i try it with the array {3,3}
    it gives me as factorial sum 7 instead of 12

    in the first element it does it correct, on the second, third, etc it just gives "1" instead of calculating it.

    I've tried changing the loop index to be until numElementB but it didn't help :(
    tbh I can't see how this gave you 7 unless you've been testing something like this {3,3,0}
    Last edited by 2by4; 12-17-2011 at 03:38 PM.

  14. #14
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    yea, it's the number i've checked.

    you see, i wrote this method that every time it sees a number that repeats itself, it makes him become (-1).
    Then i didn't know the method affects the array also in the main, now that i know that it does, i know what happend.
    when the sumfactorial method recieves the array (after it has been touched by differentDigits method) it contain (-1) in most of its elements.

    what i need is a way to make array B stay the same.
    I've found a way to do so (which i don't really like coz it takes a lot of memory), but it won't work.
    I've posted it up, so u can see. i guess i need a new loop in order to get B elements values into a new C array : \ is that correct?

  15. #15
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    Ok,
    look what I've done now:

    Java Code:
    public static int differentDigits (int[] B , int numElementsB) { //differentDigits method starts
    		
    		int[] C = new int[1000];
    		BintoC(B , C , numElementsB );
    		int count = 0; 
    		for (int i = 1 ; i < numElementsB ; i++) {
    			
    			if (C[i]>=0) count++;
    			for (int n = (i+1) ; n < numElementsB ; n++){		
    					if (C[i]==C[n]) {
    						C[n] =-1;
    					}//2st if ends
    				
    				
    			}//2st for ends
    			
    			
    			
    		}//1st for ends
    		return count;
    		
    		
    		
    	}
    
    
    	public static void BintoC (int[] B , int[] C, int numElementsB){ // BintoC method starts - copying B elements values into C. 
    		for (int i=0 ; i <= numElementsB ; i++){
    			C[i]=B[i];
    		}
    		
    		
    	}
    Well, it works fine now, but it still annoys me.
    Isn't there any other way of copying this array into the other without invoking a new method and a loop?! (i can't put 2 loops in the same method (except differentDigits, only there I'm alowed)).

    Or even better, a way of doing this without creating a new array for it :|

    I'd love to get some ideas.. meanwhile I'm gonna smash my head at the wall a little until something comes up

  16. #16
    2by4 is offline Banned
    Join Date
    Dec 2011
    Posts
    143
    Rep Power
    0

    Default Re: Factorial method bug

    Original code (with correction as specified by pbrockway2, works fine on my machine. {3, 3} gives me 12.

    Brrrr...I had to type it all in by hand for reasons to complex to get into..

  17. #17
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,565
    Rep Power
    12

    Default Re: Factorial method bug

    A few things:

    If you are going to do multiple things with the array it is (as you have found) really, really important that you don't alter the array contents. The array - to use Barbara Liskov's term - is a "shared object". It is shared between the caller and the methods. That's what makes the "return" unnecessary in the other thread. And, as I think you have also seen, it means that the methods "can look, but must not touch" when it comes to the array.

    I agree with you that making a copy of the array in the digit counting method will work, but it feels wrong. What about this: the differentDigits() method could create a small "counter" array, just 10 int elements wrong. Every time it sees a digit in the big array it increases the appropriate element in the counter array. Then to report how many different digits there are it need only count how many nonzero digits there are in the counter array. Since what you have works you might want to put this aside until later.

    When you say that your code gives the "wrong" value, you seem to be basing this on what result you get after all sorts of other methods have been at work. Clearly neither me nor 2x4 nor anybody else here knows what those other methods are. A better approach (which I tried to hint at in my first reply) is to make the methods self-contained so each can be tested and verified on it's own. And document what the method in question is supposed to do - for your benefit as well as ours.

    This approach will not make bugs go away. But it might shine light on them. In particular it might distinguish between whether a problem arises because the a method is faulty and whether the problem is that some other method has unwanted side effects.

    You say "it works now". Does that mean the digits method? the factorial? both?

  18. #18
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    Quote Originally Posted by 2by4 View Post
    Original code (with correction as specified by pbrockway2, works fine on my machine. {3, 3} gives me 12.

    Brrrr...I had to type it all in by hand for reasons to complex to get into..
    Thanks man :|

  19. #19
    Mapisto is offline Senior Member
    Join Date
    Dec 2011
    Posts
    102
    Rep Power
    0

    Default Re: Factorial method bug

    Quote Originally Posted by pbrockway2 View Post
    A few things:

    If you are going to do multiple things with the array it is (as you have found) really, really important that you don't alter the array contents. The array - to use Barbara Liskov's term - is a "shared object". It is shared between the caller and the methods. That's what makes the "return" unnecessary in the other thread. And, as I think you have also seen, it means that the methods "can look, but must not touch" when it comes to the array.

    I agree with you that making a copy of the array in the digit counting method will work, but it feels wrong. What about this: the differentDigits() method could create a small "counter" array, just 10 int elements wrong. Every time it sees a digit in the big array it increases the appropriate element in the counter array. Then to report how many different digits there are it need only count how many nonzero digits there are in the counter array. Since what you have works you might want to put this aside until later.

    When you say that your code gives the "wrong" value, you seem to be basing this on what result you get after all sorts of other methods have been at work. Clearly neither me nor 2x4 nor anybody else here knows what those other methods are. A better approach (which I tried to hint at in my first reply) is to make the methods self-contained so each can be tested and verified on it's own. And document what the method in question is supposed to do - for your benefit as well as ours.

    This approach will not make bugs go away. But it might shine light on them. In particular it might distinguish between whether a problem arises because the a method is faulty and whether the problem is that some other method has unwanted side effects.

    You say "it works now". Does that mean the digits method? the factorial? both?
    It works fine now, all of it.
    I wish i just knew from the beginning about methods changing arrays and all this stuff.. could have saved me a lot of trouble.. but that's what i get for making shortcuts.. i learn the hard way :|
    Now all i want to try and do is to simplify it and make it shorter.

    I'll also remember your comment about saparating methods and check them (or post questions about them) individually.

    Thanks again.

  20. #20
    pbrockway2 is offline Moderator
    Join Date
    Feb 2009
    Location
    New Zealand
    Posts
    4,565
    Rep Power
    12

Page 1 of 2 12 LastLast

Similar Threads

  1. Factorial beginner
    By jk91 in forum New To Java
    Replies: 4
    Last Post: 11-14-2011, 10:52 PM
  2. Factorial in java
    By spidey32 in forum New To Java
    Replies: 18
    Last Post: 04-09-2011, 01:04 PM
  3. Using method to call Factorial
    By hydride in forum New To Java
    Replies: 2
    Last Post: 03-02-2010, 02:16 AM
  4. Factorial
    By Anindo in forum New To Java
    Replies: 4
    Last Post: 07-28-2009, 09:46 PM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •