File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Beginning Java and the fly likes static insights? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of EJB 3 in Action this week in the EJB and other Java EE Technologies forum!
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "static insights?" Watch "static insights?" New topic
Author

static insights?

Jon Camilleri
Ranch Hand

Joined: Apr 25, 2008
Posts: 659

I am afraid that my function for counting cards named getWinner(), is not actually reading the Card objects that are passed to it, since I am counting the ordinal() values of static values, which will have a value in any case, whether or not they are contained in the ArrayList<Card> passed to the method getWinner()

Will you confirm my stupidity, and, suggest alternatives?




I think I have already resolved this , but have had doubts , basically because I am selectively switching between the cards in my suit.



Jon
Wouter Oet
Saloon Keeper

Joined: Oct 25, 2008
Posts: 2700

That is a whole lot of code. I don't think anybody is going to read that. Making a small selection of it will increase the number of people reading it. Anyway I did a quick scan and found that your code contain a lot of duplicate code, long methods, weird constructs (such as if(... == false)), wrong variable names (variable names shouldn't begin with an underscore) and a lot of magic numbers. I would focus of fixing that because it is more error prone, harder to comprehend and hard to read.


"Any fool can write code that a computer can understand. Good programmers write code that humans can understand." --- Martin Fowler
Please correct my English.
Ernest Friedman-Hill
author and iconoclast
Marshal

Joined: Jul 08, 2003
Posts: 24166
    
  30

Totally agree with Wouter. Note that the less code you write, the fewer bugs you'll have. Excessive wordiness just provides places for bugs to hide. As an example, that last block of code you've highlighted could equivalently be written:



and you could do the same with several of the the big switches on Rank as well.


[Jess in Action][AskingGoodQuestions]
Jon Camilleri
Ranch Hand

Joined: Apr 25, 2008
Posts: 659

Wouter Oet wrote:That is a whole lot of code. I don't think anybody is going to read that. Making a small selection of it will increase the number of people reading it. Anyway I did a quick scan and found that your code contain a lot of duplicate code, long methods, weird constructs (such as if(... == false)), wrong variable names (variable names shouldn't begin with an underscore) and a lot of magic numbers. I would focus of fixing that because it is more error prone, harder to comprehend and hard to read.


Well, you can have a look at line 163 (getWinners()..) onwards that's the gory logic bit (which I'm working on in work breaks

I was thinking of reading the "magic numbers" from a text file, however, on second thoughts it would be a pointless operation because I would store them within variables that my tiny program processes in any case.
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 36478
    
  16
Jon Camilleri wrote: . . . line 163 (getWinners()..) onwards that's the gory logic bit (which I'm working on in work breaks . . .
Not any more. Another dreadful feature of you code is the long lines, which make it too hard to read. So I have added new lines, and removed lots of unnecessary whitespace. It goes to show you that there are god reasons for this style suggestion, and this one, too.
Jon Camilleri
Ranch Hand

Joined: Apr 25, 2008
Posts: 659

Campbell Ritchie wrote:
Jon Camilleri wrote: . . . line 163 (getWinners()..) onwards that's the gory logic bit (which I'm working on in work breaks . . .
Not any more. Another dreadful feature of you code is the long lines, which make it too hard to read. So I have added new lines, and removed lots of unnecessary whitespace. It goes to show you that there are god reasons for this style suggestion, and this one, too.


Well thanks for that, I would like to note that Gary, suggests that class should be coded as follows:



This is slightly different from the recommended layout (see 1.3)

What I was worried about was whether is was more readable to include getter and setter methods that simply read from and to fields grouped with the field(s) for easier readability or with fields. What do you think?

This would be the suggested layout for classes:



Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 36478
    
  16
Style is that identifiers never begin with underscores, except for package names under certain special circumstances. Always begin identifiers with a letter; $ should be reserved for a class name generated by the compiler.

Who is Gary?

No, don't intersperse getters and setters amongst the field names.

If you are given a style guide, stick to it. It is a useful discipline learning to program according to a particular style.
Jon Camilleri
Ranch Hand

Joined: Apr 25, 2008
Posts: 659

Campbell Ritchie wrote:Style is that identifiers never begin with underscores, except for package names under certain special circumstances. Always begin identifiers with a letter; $ should be reserved for a class name generated by the compiler.

Who is Gary?
Gary Cornell and Cay S. Horstmann are the authors of [url= "http://java.sun.com/developer/Books/javaprogramming/corejavavolumeone"]Core Java Volume I and II[/url].

No, don't intersperse getters and setters amongst the field names.

If you are given a style guide, stick to it. It is a useful discipline learning to program according to a particular style.

I agree, I'm "by the book" professionally, but stick to fancy jargon when troubleshooting, and, building toy (training) programs.

 
 
subject: static insights?
 
Similar Threads
looping through an enum?
Arraylist insights
java.lang.NullPointerException
Overriding equals() method within a subclass
cannot read a Collection of a custom class as a parameter