Win a copy of Design for the Mind this week in the Design forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

static insights?

 
Jon Camilleri
Ranch Hand
Posts: 664
Chrome Eclipse IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.

 
Wouter Oet
Saloon Keeper
Posts: 2700
IntelliJ IDE Opera
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Ernest Friedman-Hill
author and iconoclast
Marshal
Pie
Posts: 24208
35
Chrome Eclipse IDE Mac OS X
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
 
Jon Camilleri
Ranch Hand
Posts: 664
Chrome Eclipse IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 48652
56
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 664
Chrome Eclipse IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 48652
56
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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
Posts: 664
Chrome Eclipse IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.

 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic