loops algorithm mistake?

[NewOrder]

i want to move a bishop diagonally from one point to another

i want to move it from (row=8,column=3) to (row=7,column=2)
the move is valid

but i want to secure it against any collision with another piece. so i am running an loop from the initial position to the next.

problem is that there is some failure.. i dont know why.

the loops are inside a method i am calling from the main method (main class).

here they are:

for(int i=rowStart;i>rowEnd;i--){ // it should reduce 8 to 7.
					for(int j=columnStart;j>columnEnd;j--){// it should reduce 3 to 2
							while(pieces[i][j]==null){						
							
								a++;/ here it should count, the number of times a square is equal to null.
							}
							if(a<size){ // if few squares equal to size (rowStart-rowEnd, in this case 1), then it should give me valid false. and return it!
								valid1=false;
							}
							else{
								valid1=true;
							}
							
								
					}
				}

am i correct or do i have a mistake?

[jon.kiparsky]

for(int i=rowStart;i>rowEnd;i--){ // it should reduce 8 to 7.
	for(int j=columnStart;j>columnEnd;j--){// it should reduce 3 to 2


		while(pieces[i][j]==null){					
			a++;// here it should count, the number of times a square is equal to null.
			}

If it starts, this while loop will run forever: you do not update the test condition.

if(a<size){ // if few squares equal to size (rowStart-rowEnd, in this case 1), then it should give me valid false. and return it!
		valid1=false;
		}
		else{
		valid1=true;
		}							
	}
}

You are setting "valid1" to a value, but not returning it.

There are probably larger errors with the logic, but that's what I can tell about it from this snippet.

[NewOrder]

sorry, jon , i didnt show you the whole method. i want to concentrate on that all loop. here is the whole method

public boolean piecesCollision(ChessPiece[][]pieces,int rowStart,int columnStart ,int columnEnd,int rowEnd) {
		
		  boolean valid1=false;
		  boolean valid2=false;
		  boolean valid3=false;
		  boolean valid4=false;
			
		  int size=rowStart-rowEnd; // suppose this is a minus fig. does the computer compute it e.g. -1
		  
			int deltaX=columnEnd-columnStart;
			deltaX=deltaX<0?-deltaX:deltaX;
			int deltaY=rowEnd-rowStart;
			deltaY=deltaY<0?-deltaY:deltaY;
			//1--
			if(deltaX<0 && deltaX<0){
				int a=0;	
				for(int i=rowStart-1;i>rowEnd;i--){
					for(int j=columnStart-1;j>columnEnd;j--){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){             // and when i come here it compares a<-1??
								valid1=false;
							}
							else{
								valid1=true;
							}
							
								
					}
				}
			}
			//2++
			if (deltaX>0 && deltaY>0){
				
				int a=0;	
				for(int i=rowStart;i<rowEnd;i++){
					for(int j=columnStart;j<columnEnd;j++){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){
								valid2=false;
							}
							else{
								valid2=true;
							}
							
								
					}
				}
				
			}
			//3-+
	       if(deltaX>0 && deltaY<0){
	   		int a=0;	
			for(int i=rowStart;i>rowEnd;i--){
				for(int j=columnStart;j<columnEnd;j++){
						while(pieces[i][j]==null ){						
						
							a++;
						}
						if(a<size){
							valid3=false;
						}
						else{
							valid3=true;
						}
						
							
				}
			}
			}
			//4+-
			if (deltaX<0 && deltaY>0){
				
				int a=0;	
				for(int i=rowStart;i<rowEnd;i++){
					for(int j=columnStart;j>columnEnd;j--){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){
								valid4=false;
							}
							else{
								valid4=true;
							}
							
								
					}
				}
				
			}
			
			
			
			
			
			
			
			

		

	
		return valid1 | valid2 | valid3 | valid4;
	}

jon. the test condition is while(pieces[][]==null).
now they wont equal null forever. cause the pieces array doesnt run forever.. it counts how many nulls there are and returns a

could something like that work?

int size=rowStart-rowEnd;

			int deltaX=columnEnd-columnStart;
			deltaX=deltaX<0?-deltaX:deltaX;
			int deltaY=rowEnd-rowStart;
			deltaY=deltaY<0?-deltaY:deltaY;
			//1--
			if(deltaX<0 && deltaX<0){
				int a=0;	
				for(int i=rowStart-1;i>rowEnd;i--){// is it better to get rid of rowStart-1 here. and write rowStart?
					for(int j=columnStart-1;j>columnEnd;j--){// is it better to get rid of columnStart-1 here. and write columnStart?
							while(pieces[i][j]==null & columnStart>columnEnd & rowStart>rowEnd){						
							        rowStart--;
                                                                columnStart--;    
								a++;
							}
		                          }
				}
			
							if(a==size){
								valid1=true;
							}
							else{
								valid1=false;
							}
					}

[jon.kiparsky]

Your revised code should exit the loop eventually, so that's better. At least, it exits as long as the bishop is moving towards lower numbers on either the vertical or the horizontal axis. If both row and column are increasing, you're still infinite.

However, it still doesn't return any value that I can see.

Quick fix:

while(pieces[i][j]==null & columnStart>columnEnd & rowStart>rowEnd){

should be

while(pieces[i][j]==null [B]&&[/B] columnStart>columnEnd [B]&&[/B] rowStart>rowEnd){

Subtle but important difference.

[NewOrder]

if(deltaX<0 && deltaX<0){
				int a=0;	
				for(int i=rowStart-1;i>=rowEnd;i--){
					for(int j=columnStart-1;j>=columnEnd;j--){
while(pieces[i][j]==null && columnStart>columnEnd && rowStart>rowEnd){						
							        rowStart--;
                                                                columnStart--;    
								a++;
							}
		                          }
				}
			
							if(a==size){
								valid1=true;
							}
							else{
								valid1=false;
							}
					}

for now i am stuck with this. damn. it wont let me move the pieces.

[jon.kiparsky]

When your methods start to get this large, you might think about decomposition and restructuring. You've learned a lot by now about what's causing you problems, now think about how things could be easier.
Some things to think about:
- Try for lots of small methods that each do one thing, rather than big methods that overlap in their function.
- If you find you're repeating lines of code, as in that big chunk you posted above, find a way to take that functionality into a method, where you can call it as needed.
- Try to solve as much as possible by delegation. "Let George do it" should be your motto.

[NewOrder]

i will try to print a
and print j or i, too.

i need to see what the values are.. i cant get what is wrong with it. cause to me it seems right

[NormR1]

Is anyone looking at the OPs code?

if(deltaX<0 && deltaX<0){

This is really careful code.
Don't trust just checking it once, do it twice to be sure.

[jon.kiparsky]

Is anyone looking at the OPs code?

Trying to avoid that. It's a bit painful.

[NewOrder]

Norm1,
dammit, i am so stupid. how could i have made that mistake.

also , should i use
maths.abs(size)
to prevent a minus value

[NewOrder]

int size=rowStart-rowEnd;
                         if(size<0){
                         size=size*(-1);

		int deltaX=columnEnd-columnStart;
			deltaX=deltaX<0?-deltaX:deltaX;
			int deltaY=rowEnd-rowStart;
			deltaY=deltaY<0?-deltaY:deltaY;
			//1--
			if(deltaX<0 && deltaY<0){
				int a=0;	
				for(int i=rowStart-1;i>=rowEnd;i--){
					for(int j=columnStart-1;j>=columnEnd;j--){
							while(pieces[i][j]==null && columnStart>columnEnd && rowStart>rowEnd){						
							        rowStart--;
                                                                columnStart--;    
								a++;
                                                        
							}
		                          }
				}

	if(a==size){
								valid1=true;
							}
							else{
								valid1=false;
							}
					}

i corrected it. but still, no solution. it wont let me inside!! :(

[NormR1]

Try debugging it by adding print outs for all the controlling variables

[NewOrder]

here you go. a whole new class and same copied code. everything is seperated. test for yourself!!
it works

public class ChessInterface1{
	public static void main(String[] arg){
		
		
	
				  
int rowStart=8;
int columnStart=3;
int columnEnd =2;
int rowEnd=7;
	int a=0;
	int size=rowStart-rowEnd;
	 if(size<0){
         size=size*(-1);
	 }
				for(int i=rowStart-1;i>=rowEnd;i--){
					for(int j=columnStart-1;j>=columnEnd;j--){
							while(columnStart>columnEnd && rowStart>rowEnd){						
							        rowStart--;
                                    columnStart--;    
							     	a++;
                                            System.out.println("comparable variable is :"+a);
                                            System.out.println("The column number is :"+j);
                                            System.out.println("The row number is :"+i);
                                            System.out.println("The size number is :"+size); 
							}
		              }
				}
				
	
	if(a==size){
		System.out.println("true");
	}
	else{
		System.out.println("false");
	}
	}
}

it should return true (put valid==true , else valid==false - in the main code, ..this is only a test)

it should return true to this method calling in the main:

if(pieces[rowStart][columnStart].piecesCollision(pieces,rowStart,columnStart ,columnEnd,rowEnd ) | pieceDevour==true){ 
				                         	   nonCollision=true;
				                            }
				                            else{
				                              nonCollision=false;
				                         	   System.out.println("Sorry, your move cant collide with another piece, try again");
				                            }

dont worry about the pieceDevour. it is irrelevant here

[NewOrder]

here you go. a whole new class and same copied code. everything is seperated. test for yourself!!
it works

public class ChessInterface1{
	public static void main(String[] arg){
		
		
	
				  
int rowStart=8;
int columnStart=3;
int columnEnd =2;
int rowEnd=7;
	int a=0;
	int size=rowStart-rowEnd;
	 if(size<0){
         size=size*(-1);
	 }
				for(int i=rowStart-1;i>=rowEnd;i--){
					for(int j=columnStart-1;j>=columnEnd;j--){
							while(columnStart>columnEnd && rowStart>rowEnd){						
							        rowStart--;
                                    columnStart--;    
							     	a++;
                                            System.out.println("comparable variable is :"+a);
                                            System.out.println("The column number is :"+j);
                                            System.out.println("The row number is :"+i);
                                            System.out.println("The size number is :"+size); 
							}
		              }
				}
				
	
	if(a==size){
		System.out.println("true");
	}
	else{
		System.out.println("false");
	}
	}
}

it should return true (put valid==true , else valid==false - in the main code, ..this is only a test)

it should return true to this method calling in the main:

if(pieces[rowStart][columnStart].piecesCollision(pieces,rowStart,columnStart ,columnEnd,rowEnd ) | pieceDevour==true){ 
				                         	   nonCollision=true;
				                            }
				                            else{
				                              nonCollision=false;
				                         	   System.out.println("Sorry, your move cant collide with another piece, try again");
				                            }

dont worry about the pieceDevour. it is irrelevant here

[NormR1]

Glad to hear you are making progress.

[NewOrder]

Norm i am making no progress. i am stuck. the code seems to be correct , but it wont return true. why?

[jon.kiparsky]

I don't think this is causing your problem, but your return statement is doing bitwise comparisons of your booleans. Try logical comparisons:

return valid1||valid2|| ....

As to why you're not getting the results you want, I'll say it again and then I'll shut up: this method is too big and too complicated for you to debug it effectively, and it's too big for anyone sensible to want to leap into it.
Sub things out, learn how to use methods effectively. Think of the overall logical structure: what are the steps that you're trying to achieve with this code? When you understand the steps, they become methods. If the methods are too big, they then get reduced until you have a set of methods that are simple enough that each one's purpose and means of execution is easiy understood. Then you'll have control of your program.

[NewOrder]

it isnt, cause if you look at valid 1. it has that 2 loops. say valid2 is wrong, and i cant debug it. still , i am testing valid1 only. valid1 as you say. i debugged it (you can test it , it works) but valid

Thing of a campus , that shows north, west, east, south. what i try to estimate is where the bishop goes. if it goes east, you use the same method but with minus signs instead of pluses in some places in the loops. Thats all. i copied and pasted the same loops 3 times. when the method gets the row and column variables it decides which one of the loops to use (more accurately, it is the type of if-statements.
here look at the whole method. you will see that 50% of it is dublicated.

public boolean piecesCollision(ChessPiece[][]pieces,int rowStart,int columnStart ,int columnEnd,int rowEnd) {
		
		  boolean valid1=false;
		  boolean valid2=false;
		  boolean valid3=false;
		  boolean valid4=false;
			
		         int size=rowStart-rowEnd;
                         if(size<0){
                         size=size*(-1);
}
                           
			int deltaX=columnEnd-columnStart;
			deltaX=deltaX<0?-deltaX:deltaX;
			int deltaY=rowEnd-rowStart;
			deltaY=deltaY<0?-deltaY:deltaY;
			//1--
			if(deltaX<0 && deltaY<0){
				int a=0;	
				for(int i=rowStart-1;i>=rowEnd;i--){
					for(int j=columnStart-1;j>=columnEnd;j--){
							while(pieces[i][j]==null && columnStart>columnEnd && rowStart>rowEnd){						
							        rowStart--;
                                                                columnStart--;    
								a++;
                                                         
							}
		                          }
				}
			
							if(a==size){
								valid1=true;
							}
							else{
								valid1=false;
							}
					}		
								
			
			//2++
			if (deltaX>0 && deltaY>0){
				
				int a=0;	
				for(int i=rowStart;i<rowEnd;i++){
					for(int j=columnStart;j<columnEnd;j++){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){
								valid2=false;
							}
							else{
								valid2=true;
							}
							
								
					}
				}
				
			}
			//3-+
	       if(deltaX>0 && deltaY<0){
	   		int a=0;	
			for(int i=rowStart;i>rowEnd;i--){
				for(int j=columnStart;j<columnEnd;j++){
						while(pieces[i][j]==null ){						
						
							a++;
						}
						if(a<size){
							valid3=false;
						}
						else{
							valid3=true;
						}
						
							
				}
			}
			}
			//4+-
			if (deltaX<0 && deltaY>0){
				
				int a=0;	
				for(int i=rowStart;i<rowEnd;i++){
					for(int j=columnStart;j>columnEnd;j--){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){
								valid4=false;
							}
							else{
								valid4=true;
							}
							
								
					}
				}
				
			}
			
			
			
			
			
			
			
			

		

	
		return (valid1 || valid2 || valid3 || valid4);
	}






}

[jon.kiparsky]

here look at the whole method. you will see that 50% of it is dublicated.

Yes, that's a big part of the problem. Why do you write the same code over and over?

[NewOrder]

a more important question. in eclipse when i erase the boolean valid in the return. it tells me that boolean isnt used. but it is used. could it be that my boolean isnt seen.
look at the two remarks in the code:

boolean valid1=false; // if the valid from the return statement is removed . it says here that this variable isnt used. but why. i do use it in the loops. could it be that its visibility damaged?
		  boolean valid2=false;
		  boolean valid3=false;
		  boolean valid4=false;
			
		         int size=rowStart-rowEnd;
                         if(size<0){
                         size=size*(-1);
}
                           
			int deltaX=columnEnd-columnStart;
			deltaX=deltaX<0?-deltaX:deltaX;
			int deltaY=rowEnd-rowStart;
			deltaY=deltaY<0?-deltaY:deltaY;
			//1--
			if(deltaX<0 && deltaY<0){
				int a=0;	
				for(int i=rowStart-1;i>=rowEnd;i--){
					for(int j=columnStart-1;j>=columnEnd;j--){
							while(pieces[i][j]!=null && columnStart>columnEnd && rowStart>rowEnd){						
							        rowStart--;
                                    columnStart--;    
								    a++;
                                                        
							}
		                          }
				}
			
							if(a==size){
								valid1=true;
							}
							else{
								valid1=false;
							}
					}		
								
			
			//2++
			if (deltaX>0 && deltaY>0){
				
				int a=0;	
				for(int i=rowStart;i<rowEnd;i++){
					for(int j=columnStart;j<columnEnd;j++){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){
								valid2=false;
							}
							else{
								valid2=true;
							}
							
								
					}
				}
				
			}
			//3-+
	       if(deltaX>0 && deltaY<0){
	   		int a=0;	
			for(int i=rowStart;i>rowEnd;i--){
				for(int j=columnStart;j<columnEnd;j++){
						while(pieces[i][j]==null ){						
						
							a++;
						}
						if(a<size){
							valid3=false;
						}
						else{
							valid3=true;
						}
						
							
				}
			}
			}
			//4+-
			if (deltaX<0 && deltaY>0){
				
				int a=0;	
				for(int i=rowStart;i<rowEnd;i++){
					for(int j=columnStart;j>columnEnd;j--){
							while(pieces[i][j]==null){						
							
								a++;
							}
							if(a<size){
								valid4=false;
							}
							else{
								valid4=true;
							}
							
								
					}
				}
				
			}
			
			
			
			
			
			
			
			

		
			return valid1 | valid2 | valid3 | valid4; // if i remove one of the valid statements, it says taht the boolean valid=false; isnt used