This week's book giveaway is in the HTML Pages with CSS and JavaScript forum.
We're giving away four copies of Testing JavaScript Applications and have Lucas da Costa on-line!
See this thread for details.
Win a copy of Testing JavaScript Applications this week in the HTML Pages with CSS and JavaScript forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Bear Bibeault
  • Ron McLeod
  • Jeanne Boyarsky
  • Paul Clapham
Sheriffs:
  • Tim Cooke
  • Liutauras Vilda
  • Junilu Lacar
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • fred rosenberger
  • salvin francis
Bartenders:
  • Piet Souris
  • Frits Walraven
  • Carey Brown

TicTacToe 2D Array

 
Greenhorn
Posts: 6
C++ Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,

I'm completing an assignment for class to create a 2 player tic-tac-toe game using 2D arrays. I've written a lot of the code and I'm wondering if there's any way to improve my won method. Also, I'm not sure how to start up the game in the main method. Normally, I'd have multiple classes and create an instance of the game in a final driver class but for this lab, everything had to be in one class. Right now it's just looping the output of "We have a winner. Player 2 wins" without letting me input anything.

 
Bartender
Posts: 7208
65
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows
 
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch!

It's great that you are already looking for ways to improve your code. And yes, there is definitely a lot of room for improvement in the organization of your code.

Let's start with what you've done well.

1. You have some method names that are really well chosen: isBoardFull(), move(), won(), and printBoard().  Those method names express their intent well without saying much about the implementation underneath. This is a very good thing.
2. You seem to have a good natural sense of organization. You have broken down the program to some logical chunks and put related concerns together inside their own methods.

However, your code is still quite "smelly," as we say in the industry. That means there are problems that make it difficult to understand and work with.

Here are a few of the smells, with the stronger smells listed higher:

1. Long methods
2. Mixing of concerns - display statements are mixed in with game logic
3. Duplicate code
4. Leaky abstractions - names that give away implementation details
5. Non-object-oriented code (all static methods)

Which of the above smells do you think your won() method suffers from? (Hint: there are more than one)
 
Junilu Lacar
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

I wrote:Which of the above smells do you think your won() method suffers from?


Carey Brown wrote:Your char array will hold '-', 'x', or 'o'. Your won() method only tests for 'x'.



Well, besides the smell that it doesn't work half the time, that is.  

One more option to choose:
1. It's difficult to localize bugs.
 
Max Brown
Greenhorn
Posts: 6
C++ Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for replying!

I see what you mean by smelly code. First of all, all my methods are static. I was under the impression that because they're running in the static main method, they'd have to be static as well. I'll look into static methods more. Then, I have a lot of display messages and some duplicate. I'm not quite sure what you mean by leaky abstractions though.
I feel like it should be shorter but I'm not sure how to go about doing so. Right now my logic is to check the rows, columns, then both diagonals.
 
Junilu Lacar
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
To get you started with the end in mind, here's how I might have (eventually) written something like the won() method:

I say "(eventually)" because I would have started with code that had similar smells to what you have. However, through the process of refactoring, I would transform the code so that it looks like I actually knew what I was doing in the first place.

Most good code doesn't get that way right off the bat. Most good code started out as smelly code. Then, through a series of small tweaks (refactoring), it slowly gets better and better. Listing 1 above is a great example. As I wrote this reply, I changed that snippet several times before hitting the submit button. It actually looked like this the first time I wrote it:

The code in Listing 2 was transformed into the code in Listing 1 after several applications of the Rename refactoring
 
Junilu Lacar
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Max Brown wrote:I'm not quite sure what you mean by leaky abstractions though.


When not sure about something, Google. What is a leaky abstraction?

Or you could come here and ask the nice people who hang out at the Ranch

As far as I know, the term "leaky abstraction" was coined by a really smart guy named Joel Spolsky. Go out there and read some of his articles because there's a lot of good stuff he's written that I think all programmers should know.

Anyway, a leaky abstraction is an expression of an abstract idea that reveals (leaks) details of the underlying implementation. For example, the method initializeArray() reveals that there is an array involved (an implementation detail). A better name for that method might be clearBoard(). That name expresses the intent rather the implementation.
 
Junilu Lacar
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Again, starting with the end in mind, the wonOnDiagonal() method I would eventually write might look something like this:

 
Junilu Lacar
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I hope you've noticed something about the code snippets I've given so far.

They all express pure intent and show no implementation details whatsoever.
 
Junilu Lacar
Sheriff
Posts: 15815
264
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Max Brown wrote:I was under the impression that because they're running in the static main method, they'd have to be static as well.


They're not running in the static main method though. You are calling them from the static main. In technical terms, you are trying to invoke that functionality from a static context, thus the code also needs to be in a static context. It doesn't have to be.

The main() method can look like this:

If you start with the above code, your play() method might look a little like this:

I'll look into static methods more.


Actually, I hope you stop looking at static methods and start looking at instance methods more instead. Like the play() method above, for example.
 
Marshal
Posts: 69894
278
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Max Brown wrote:. . . . I was under the impression that because they're running in the static main method, they'd have to be static as well. . . .

Did you get that impression because you suffered a compiler error message saying, “non‑static”? I think the wording of that error message confuses lots of people.
 
A teeny tiny vulgar attempt to get you to buy our stuff
Thread Boost feature
https://coderanch.com/t/674455/Thread-Boost-feature
    Bookmark Topic Watch Topic
  • New Topic