Meaningless Drivel is fun!*
The moose likes Beginning Java and the fly likes How do  i improve variable visibility? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "How do  i improve variable visibility?" Watch "How do  i improve variable visibility?" New topic
Author

How do i improve variable visibility?

Dmitri Makovetskiy
Ranch Hand

Joined: Jun 21, 2010
Posts: 128

i have this code. the problem is that it returns on each boolean (valid5,6,7,8,) true. like no matter what i send to this method.

basically, i am trying to move a rook, without colliding with other pieces.

i debugged every loop & condition twice and it works. The problem is that the result of valid isnt seen or returned correctly in the return statment at the bottom.

help. ?!

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39834
    
  28
This code is difficult to understand, but on a 2-second look I can see a few problems.

A styling problem. Don't saySay
In your loop, you are incrementing both parts of the index an adding them, so you are going along the column/row two squares at a time. You appear to have similar errors in all of your loops.

There is bound to be more that I haven't noticed.
Wouter Oet
Saloon Keeper

Joined: Oct 25, 2008
Posts: 2700

I've got some comments about the quality of your code. Usually if the quality is bad then problems are much harder to spot.

Some of your local variables start with an uppercase character. Only class names should start with that.
valid5-8 are not descriptive names. Why not call them canMoveUp, canMoveDown etc.
Why are you passing an ChessPiece[][]? It's this a prime candidate for a instance variable?
You have 4 "blocks" of code. Why not break them up into small methods.
You could replace some of your conditional logic with a method.
I personally don't like to change the values of parameters because it can cause confusion and therefor declare them final but that is personal.

You're incrementing the weird-named variable b though out the whole method. Are you sure you want this?


"Any fool can write code that a computer can understand. Good programmers write code that humans can understand." --- Martin Fowler
Please correct my English.
Dmitri Makovetskiy
Ranch Hand

Joined: Jun 21, 2010
Posts: 128
Wouter Oet wrote:I've got some comments about the quality of your code. Usually if the quality is bad then problems are much harder to spot.

Some of your local variables start with an uppercase character. Only class names should start with that.
valid5-8 are not descriptive names. Why not call them canMoveUp, canMoveDown etc.
Why are you passing an ChessPiece[][]? It's this a prime candidate for a instance variable?
You have 4 "blocks" of code. Why not break them up into small methods.
You could replace some of your conditional logic with a method.
I personally don't like to change the values of parameters because it can cause confusion and therefor declare them final but that is personal.

You're incrementing the weird-named variable b though out the whole method. Are you sure you want this?





Thank you . both of you spotted the mistakes. i am going to correct all what i did wrong and submit my code again. in 1-2 hours approximat.



here is the code. is it any better. i applied same principles and it works it returns value. why did my last code made problems?
but first, do you think , i debugged the thing correctly?

Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 39834
    
  28
Dmitri Makovetskiy wrote: . . . Thank you
You're welcome
both of you spotted the mistakes. . . .
No, we spotted some of them.

I don't know whether the code is any better. I haven't even looked.

You will have to work out what you want from your code, and how to obtain it. I keep telling our undergraduates they require thee pieces of hardware, and I think you need the same:
  • A pencil
  • Some paper
  • An eraser. Preferably a large one

  • You write down (in words of one syllable if possible) what ought to happen.
  • 1: Look up the rank (row).
  • 2: Go one square at a time.
  • 3: See if there is a piece on that square or not.
  • When you have written everything simply, you will see what your method ought to do. And you will have pseudo-code to implement that method from.
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Campbell Ritchie wrote:
    Dmitri Makovetskiy wrote: . . . Thank you
    You're welcome
    both of you spotted the mistakes. . . .
    No, we spotted some of them.

    I don't know whether the code is any better. I haven't even looked.

    You will have to work out what you want from your code, and how to obtain it. I keep telling our undergraduates they require thee pieces of hardware, and I think you need the same:
  • A pencil
  • Some paper
  • An eraser. Preferably a large one

  • You write down (in words of one syllable if possible) what ought to happen.
  • 1: Look up the rank (row).
  • 2: Go one square at a time.
  • 3: See if there is a piece on that square or not.
  • When you have written everything simply, you will see what your method ought to do. And you will have pseudo-code to implement that method from.


    Ritchie ,
    first, i use pencil and paper all the time. cause i feel i am about to finish the code, but there is some bug somewhere , and i have been searching for it for days.
    second, if you copy , paste and run the code. it tells you everything that is happening at each stage. you could set the columnStart & columnEnd to different values. and it works

    now i am going to try to understand , what my older code didnt have. why doesnt it deliver the code:

    i have a visibilty question if i wrap the whole loop in an if statment would the boolean value be retained?

    Wouter Oet
    Saloon Keeper

    Joined: Oct 25, 2008
    Posts: 2700

    You're welcome.

    A good test for code quality is to imagine if you can easily comprehend the code after a period of 3 months doing something else.
    In this case it would say no. Follow the method that Campbell described and it will be much easier to comprehend.
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Wouter Oet wrote:You're welcome.

    A good test for code quality is to imagine if you can easily comprehend the code after a period of 3 months doing something else.
    In this case it would say no. Follow the method that Campbell described and it will be much easier to comprehend.


    Oet it is a project that i am going to submit next week. i remember everything that i wrote. plus it is an easy code. only a loop inside an if question.
    could someone answer regarding my previous question above (visibility)?

    could you see if there are problems with my code. i corrected and debugged it as much as i could!!

    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39834
        
      28
    Dmitri Makovetskiy wrote: . . . could you see if there are problems with my code. . . .
    That isn't what we are here for.
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39834
        
      28
    Dmitri Makovetskiy wrote: . . . if you copy , paste and run the code. it tells you everything that is happening at each stage. . . .
    No, it won't because it is only one method. What is actually going wrong? Have you followed the flow of execution? Have you tried a debugger like in Eclipse? That will allow you to follow the code, execute a single line at a time, and inspect all the values of fields and local variables. Then you can really see what is happening.
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Campbell Ritchie wrote:
    Dmitri Makovetskiy wrote: . . . if you copy , paste and run the code. it tells you everything that is happening at each stage. . . .
    No, it won't because it is only one method. What is actually going wrong? Have you followed the flow of execution? Have you tried a debugger like in Eclipse? That will allow you to follow the code, execute a single line at a time, and inspect all the values of fields and local variables. Then you can really see what is happening.





    the question is this.
    suppose that i have got
    rowstart=8
    rowend=6
    columnstart=8
    columnend=8

    b=0


    would be be 2?
    Wouter Oet
    Saloon Keeper

    Joined: Oct 25, 2008
    Posts: 2700

    Dmitri Makovetskiy wrote:it is a project that i am going to submit next week. i remember everything that i wrote. plus it is an easy code. only a loop inside an if question.
    What if your teacher a month from now says that it's not good enough and that you should improve it. Sure you can remember it now but not in a month or 3 months. Or when a classmate wants to see it to compare with his/her code.

    Your boolean will be visible if you declare it outside the if-statement.
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Wouter Oet wrote:
    Dmitri Makovetskiy wrote:it is a project that i am going to submit next week. i remember everything that i wrote. plus it is an easy code. only a loop inside an if question.
    What if your teacher a month from now says that it's not good enough and that you should improve it. Sure you can remember it now but not in a month or 3 months. Or when a classmate wants to see it to compare with his/her code.

    Your boolean will be visible if you declare it outside the if-statement.



    i debugged the whole thing. everything is same, the only thing that is different is the type of array. in my test code it is string and in my real code it is object:
    what do you think?



    The problem is when i put it in the main..it wont work
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39834
        
      28
    What is going to happen when you try to access pieces[0][0]?
    If you move a rook, surely isMovingLeft || isMovingRight || isMovingUp || isMovingDown will always evaluate to true?
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Campbell Ritchie wrote:What is going to happen when you try to access pieces[0][0]?
    If you move a rook, surely isMovingLeft || isMovingRight || isMovingUp || isMovingDown will always evaluate to true?



    I HAVE NO PIECES[0][0]

    and i took care of 0 b 0 columnY or rowX

    by this




    i am actually stuck cause the code above works

    and the code with the chessobjects doesnt..





    here its array:
    >
    Wouter Oet
    Saloon Keeper

    Joined: Oct 25, 2008
    Posts: 2700

    If your other code is working then just compare it. However I still think that you should refactor. Clean your code.
    Like Campbell said:
    Campbell Ritchie wrote:Don't saySay


    And why do you keep posting code with a horrible layout?
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Wouter Oet wrote:If your other code is working then just compare it. However I still think that you should refactor. Clean your code.
    Like Campbell said:
    Campbell Ritchie wrote:Don't saySay


    And why do you keep posting code with a horrible layout?


    Oet it isnt an issue why the layout bad. cause i import from eclipse, thats the answer!!!

    This code works
    (non chess piece code)

    you could actually change the variables column , row values, string array[][], values. and see that it works. try it yourself.

    when i import it to my chesspiece code . it doesnt check for null i.e. doesnt work. i dont know why. and it is beyond my conception of how the chess piece array is being scanned or imported to the method.



    i simply copied and pasted the working code (after it was debugged), exactly as it is to the chess piece code
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Wouter Oet wrote:If your other code is working then just compare it. However I still think that you should refactor. Clean your code.
    Like Campbell said:
    Campbell Ritchie wrote:Don't saySay


    And why do you keep posting code with a horrible layout?


    Wouter Oet wrote:If your other code is working then just compare it. However I still think that you should refactor. Clean your code.
    Like Campbell said:
    Campbell Ritchie wrote:Don't saySay


    And why do you keep posting code with a horrible layout?


    Oet it isnt an issue why the layout bad. cause i import from eclipse, thats the answer!!!

    This code works
    (non chess piece code)

    you could actually change the variables column , row values, string array[][], values. and see that it works. try it yourself.

    when i import it to my chesspiece code . it doesnt check for null i.e. doesnt work. i dont know why. and it is beyond my conception of how the chess piece array is being scanned or imported to the method.



    i simply copied and pasted the working code (after it was debugged), exactly as it is to the chess piece code
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39834
        
      28
    Dmitri Makovetskiy wrote:. . . cause i import from eclipse . . .
    Does Eclipse really format your code like that? You ought to change your configuration, then.
    Wouter Oet
    Saloon Keeper

    Joined: Oct 25, 2008
    Posts: 2700

    If eclipse formats your code that way you should really change your configuration. Or at least fix your post so that we can read it in a normal way.

    You keep saying that there is no problem with the layout/coding but I say that there is. It makes it so much harder to read and much easier for mistakes to slip in. You're using bitwise & where the logical && should be used. You have if-statements that check conditions that are already checked. You have variables with weird names. You have those weird if(boolean) someBoolean = false; else someBoolean = true; blocks. You have a fairly big method that can easily be broken up into smaller, more comprehensible methods. And last but not least you keep saying that is doesn't work but WHAT doesn't work?
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Wouter Oet wrote:If eclipse formats your code that way you should really change your configuration. Or at least fix your post so that we can read it in a normal way.

    You keep saying that there is no problem with the layout/coding but I say that there is. It makes it so much harder to read and much easier for mistakes to slip in. You're using bitwise & where the logical && should be used. You have if-statements that check conditions that are already checked. You have variables with weird names. You have those weird if(boolean) someBoolean = false; else someBoolean = true; blocks. You have a fairly big method that can easily be broken up into smaller, more comprehensible methods. And last but not least you keep saying that is doesn't work but WHAT doesn't work?



    let me explain myself in a simple manner.

    the code above works (it is a debugged code with one class only)

    i transfered all its body apart from its String array to my original chess piece code, that has a method which is exactly the same. checks if there are nulls.

    what i think is the problem, is in the heading of my original method

    public boolean piecesCollision(ChessPiece[][]pieces,int rowStart,int columnStart ,int columnEnd,int rowEnd) {

    when i send pieces arguments to ChessPiece[][]pieces, i think that pieces arent applied correctly in the method. cause when i scan pieces with my loop/s , it doesnt stop when there is an object..it continues counting all the array places as if they were all null.

    Oet please be on track. try to comment on the general method executiion instead of nittpicking how i write things

    to tell you the truth, Oet, i doubt that you will track the problem, i think that the mistake is embedded somewhere on the margins on this whole code..somewhere not so deep inside the algorithm of this code
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39834
        
      28
    Dmitri Makovetskiy wrote: . . . Oet please be on track. try to comment on the general method executiion instead of nittpicking how i write things
    I beg your pardon? That is no way to talk to another ranch member.
    Wouter Oet
    Saloon Keeper

    Joined: Oct 25, 2008
    Posts: 2700

    Excuse me? I'm making an effort and spending part of my free time to help you out. If you had made the changes to your code that Campbell and I suggested then this would have been fixed hours ago. I'm not nitpicking at all. These are serious problems that jeopardize the success of your code.

    I wish you the best of luck with your code because I'm going to help people that do value my input.
    Dmitri Makovetskiy
    Ranch Hand

    Joined: Jun 21, 2010
    Posts: 128
    Wouter Oet wrote:Excuse me? I'm making an effort and spending part of my free time to help you out. If you had made the changes to your code that Campbell and I suggested then this would have been fixed hours ago. I'm not nitpicking at all. These are serious problems that jeopardize the success of your code.

    I wish you the best of luck with your code because I'm going to help people that do value my input.



    Oet, i checked the code with one of my friends , it works. the algorithm is correct.. the method fails to evaluate my chess objects i.e. ChessInterface [][] pieces=new ChessInterface[9][9] (i know chessboard is 8 by 8, it isnt relevant.)

    i used String [][] pieces=new String[8][8]; in my debugged code
    (9 or 8 has no effect on the workability of the code. i checked)
    and it worked, but the original code didnt. i am not conceptually aware to why?
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 39834
        
      28
    You needn't expect an answer until you apologise to Wouter.
    David Newton
    Author
    Rancher

    Joined: Sep 29, 2008
    Posts: 12617

    Dmitri Makovetskiy wrote:try to comment on the general method executiion instead of nittpicking how i write things

    What you don't seem to understand is that the harder you make it for other people to read your code the less likely anyone will take the time to help you.

    When I ask people for help I try really, really hard to conform to what they expect of me: this has several advantages.

    1) They see that I care about what they think
    2) They know things I don't, so it's important to me that they understand I am willing to put in the effort to learn from them
    3) It makes it as easy as possible for them to be *able* to help me

    Poorly-formatted code is difficult to reason about, because we're trying to decipher not only the algorithm, but the mechanical aspects of the code itself. Reading code is difficult: even simple code takes time to reason about. *Any* amount of complication above that has two effects: 1) it makes it increasingly unlikely that people will take the time to help, and 2) even if the time is taken, it's less likely that the code is understandable in such a way that it's *possible* to help.

    It's not a frivolous request, and it's in your own best interest to comply: if we can't make heads or tails of your code, we simply can't help. How much time do you expect us to invest in what is, ultimately, *your* homework, and *your* learning experience? The amount of time we've taken to date should surely indicate that we're willing, able, and capable of helping--but if you make it too difficult, or can't make the simplest of changes to help us help you... why should we expend more effort than you're willing to?
     
    wood burning stoves
     
    subject: How do i improve variable visibility?