• 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 Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Clean code

 
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I have a baseball program which has a Team object and the team object has a playInning method. The playInning method is going to call a series of methods to "play" the inning and the methods will need access to several variables (number of outs, who is on what base, etc). Which is considered cleaner code, making the needed variables fields in the team object so all of the methods have access to it, or making a new object (an inner class?) with all of the needed variables in it and just passing the new object to the variables? Or is there a better way than either of these? Is there a design pattern which covers this I should look at? Any help would be appreciated. Thanks!
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The principle/law that comes to mind is the Law of Demeter. It's easier to evaluate whether code is clean or not if we can see actual code and not just an abstract description as you've provided here. Code is precise and you see exactly what you're getting. High-level descriptions leave too much to the imagination, IMO.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks! I will post it when I get it cleaned up and will welcome comments.
 
Marshal
Posts: 79151
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Post it now and get help cleaning it up.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Here is the code after my first try at cleaning it up. I did not include all of the player class, just the the method playInning uses. The player object contains the percentage of times the player hits a single, double, etc. and player.batterResult() uses a random number generator to return a legal result of an at bat. The team is just an ArrayList of player objects. In Eclipse all the enums and classes are in their own file. If anyone has time to look at this and make suggestions it will be greatly appreciated!

 
Saloon Keeper
Posts: 10687
85
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows ChatGPT
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I'm still looking. Your enum Bases doesn't need the int as this is already provided by enum's as an ordinal.
 
Bartender
Posts: 1845
10
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
To me it looks like overcomplicated because every time you are doing anything (moving bases, counting runs etc) you have to look at the who is on what base (I don't know), and what the batter did.
That logic gets complicated because of the combinations possible.
I would try to see how to simplify things.

Specifically I don't like the way the bases are updated after a hit.
You have 5 methods to do this
advanceBasesForWalk
advanceBasesForSingle
advanceBasesForDouble
advanceBasesForTriple
advanceBasesForHomeRun

I would think it would be simple with an
"advanceBases" method which advances all the base positions by one. And to advance for a double, you call this method twice. etc etc


Your calculation on runs suffers from the same thing. You analyse who is on the bases, and how hard the batter hit the ball to work out if anybody got home to score. And that may be 1, 2 or 3
As an alternative, why not run the scoring out of "advanceBases" so that any time somebody moves from 3rd base to home, you add a run.

Just my $.02.
 
Carey Brown
Saloon Keeper
Posts: 10687
85
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows ChatGPT
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You've put a lot of stuff into Team that doesn't seem specific to a "Team" object. Some of the fields should be moved off to a Game class: bases, playersCanSteal, outs, runs, etc.. A Game would essentially keep track of the state of a game. A Game would have some method that takes an event, which might be the type of hit the batter made or it might include other things as well. The Game's state would change based on the event received. You might also want a Statistics class.
 
Carey Brown
Saloon Keeper
Posts: 10687
85
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows ChatGPT
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Not sure why you need a copy constructor.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Smells:

1. The advanceRunnersForXXX methods smack of duplication.
2. Can't really find a coherent "story" to follow here. Good designs/code should read like prose: there is a beginning and end and a natural flow between those two points.
3. Responsibility assignments seem strange. Why does a Player have batterResult? And look at Team, most of the methods do not seem to be responsibilities of a "Team"
4. No tests

Try this: relate what you want the program to do as a story. For example:

I want to simulate a baseball game. I want to set up two teams. Play will be simulated randomly. Sample output:

Simulation:

Inning: 1
Team 1 at bat
Player 1 hits a SINGLE
Player(s) on FIRST
Player 2 hits a DOUBLE
Player(s) on SECOND, THIRD
Player 3 walks
Bases are loaded
Player 4 hits a SINGLE
Player 1 scores
Player 5 strikes out
and so on...

 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Stefan Evans wrote:
I would think it would be simple with an
"advanceBases" method which advances all the base positions by one. And to advance for a double, you call this method twice. etc etc


Your calculation on runs suffers from the same thing. You analyse who is on the bases, and how hard the batter hit the ball to work out if anybody got home to score. And that may be 1, 2 or 3
As an alternative, why not run the scoring out of "advanceBases" so that any time somebody moves from 3rd base to home, you add a run.

Just my $.02.



I couldn't figure out a way to use an advanceBases method because everyone doesn't always move up one base. A man on second will move two bases on a single and not at all on a walk with first base open so you have to check to see who is on what base.

I did have the scoring in advanceBases but that violated the one method does one thing rule so I split it up. By the way, thanks for the input.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Carey Brown wrote:Not sure why you need a copy constructor.



One of the options in the main program is to use an existing team and make some changes in it and then play a game. It took me a lot of google searching to figure out why both teams always looked the same! So I needed the copy constructor to make an independent copy of a team.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Carey Brown wrote:I'm still looking. Your enum Bases doesn't need the int as this is already provided by enum's as an ordinal.



Could you show me how I would change the code to use that? Thanks!
 
Carey Brown
Saloon Keeper
Posts: 10687
85
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows ChatGPT
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Kendall Ponder wrote:

Carey Brown wrote:I'm still looking. Your enum Bases doesn't need the int as this is already provided by enum's as an ordinal.



Could you show me how I would change the code to use that? Thanks!


 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
One common pitfall is trying to model the program too closely to the real world. While it helps when you can draw parallels between the abstract software object relationships and real-world relationships, sometimes it makes more sense to have objects that are simply abstractions. For example, you have the concept of an AtBatResult. It seems to me that it would make more sense to have a class named TurnAtBat be responsible for generating the AtBatResult. You could have code like this:

This seems like plausible code and it actually tells a small story. When I design, I try to write the code that I'd like to see first. Then I start writing tests to drive my design thinking down that road and see where it leads me and what kind of concepts present themselves. When the "story" starts becoming confused, that's when I start looking for smells and opportunities to refactor for clarity.

Note that the above may not be the code I end up with when I'm done but it's a starting point for exploring different ways of telling a story. I actually spend more time reading and re-reading my code than I do writing it. Also, I don't try to tell the whole story all at once. I start from a core idea, then work my way outward, adding more details, reading and re-reading to make sure what I have so far still hangs together cohesively and coherently.


 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Carey Brown wrote:You've put a lot of stuff into Team that doesn't seem specific to a "Team" object. Some of the fields should be moved off to a Game class: bases, playersCanSteal, outs, runs, etc.. A Game would essentially keep track of the state of a game. A Game would have some method that takes an event, which might be the type of hit the batter made or it might include other things as well. The Game's state would change based on the event received. You might also want a Statistics class.



So the playInning method would belong to the Game class (I have one) which would take two teams as parameters. Then I would copy the teams to local variables and all of the code I have in the Team object would be in the Game class and Team would just be the ArrayList with getters & setters. That makes sense. Does that move the details of the program higher up the hierarchy? What would you recommend I put in the Statistics class?
 
Marshal
Posts: 4491
572
VSCode Eclipse IDE TypeScript Redhat MicroProfile Quarkus Java Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Carey Brown wrote:

Kendall Ponder wrote:

Carey Brown wrote:I'm still looking. Your enum Bases doesn't need the int as this is already provided by enum's as an ordinal.



Could you show me how I would change the code to use that? Thanks!



Another approach would be to use a Map to keep track of who is on each base;
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Seems to me that a more natural data structure to represent the bases would be a queue (FIFO). If you see the bases, include home, as spots in a 4-item queue, it seems it's easier to just advance each item (a Player) along in that queue as a result of whatever play is simulated. If a Player gets to the end of the queue without getting tagged out, he scores. If a Player is tagged out at one of the bases, then he gets removed from the queue. You also need to track the players who are back in the batting lineup, which I suppose would have to be some kind of revolving queue. The player who is at the head of the queue goes up to bat and is either put into the bases queue or cycled back to the end of the batting lineup queue. When a Player scores and gets out of the bases queue, he will need to be reinserted in his proper spot in the batting lineup queue.

This might also help simplify the logic for simulating bases being stolen, double plays, triple plays, grand slams, etc.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Smells:

1. The advanceRunnersForXXX methods smack of duplication.
2. Can't really find a coherent "story" to follow here. Good designs/code should read like prose: there is a beginning and end and a natural flow between those two points.
3. Responsibility assignments seem strange. Why does a Player have batterResult? And look at Team, most of the methods do not seem to be responsibilities of a "Team"
4. No tests

Try this: relate what you want the program to do as a story. For example:

I want to simulate a baseball game. I want to set up two teams. Play will be simulated randomly. Sample output:

Simulation:

Inning: 1
Team 1 at bat
Player 1 hits a SINGLE
Player(s) on FIRST
Player 2 hits a DOUBLE
Player(s) on SECOND, THIRD
Player 3 walks
Bases are loaded
Player 4 hits a SINGLE
Player 1 scores
Player 5 strikes out
and so on...



1. I agree it smacks of duplication but for reasons I pointed out above I couldn't find a way around it.

2. My attempt at a story was:


The player bats and depending on what he does, runs are scored, runners are advanced, outs are updated and then the next batter comes up. Steals occur during an at bat in real life but they always occur before the result of the batter which means they occur between batters.

3. My thought is a player performs an atBat and a team plays an inning just like a Car stops so a player should have an batterResult method like a Car object should have a stop method. Plus, if I put the batterResult method in game doesn't that couple my Game class more closely with my Team and Player classes? If I decide to take into account sacrifice flies and change the Player object accordingly it wouldn't effect anyone who has used my playInning method to run their own game simulations. If the logic is all in my Game class then doesn't that limit flexibility? Or is it better to not have any methods in a data class?

4. I don't understand what you mean by no tests. Are you talking about debugging?
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Ron McLeod wrote:

Carey Brown wrote:

Kendall Ponder wrote:

Carey Brown wrote:I'm still looking. Your enum Bases doesn't need the int as this is already provided by enum's as an ordinal.



Could you show me how I would change the code to use that? Thanks!



Another approach would be to use a Map to keep track of who is on each base;



Thanks! I haven't used maps before.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Ron McLeod wrote:
Another approach would be to use a Map to keep track of who is on each base;


Where would these methods be called from and in what class would these methods belong? My Spidey senses are tingling from reading this and they're telling me that the design thinking is focusing on the small and may be losing sight to the big picture. Who's keeping track of the base? Why is player an int? What is the type of playerOnBase? (Edit: sorry, I somehow overlooked the declaration. But that kind of tells me that the Map is not a very natural fit for this)
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:One common pitfall is trying to model the program too closely to the real world. While it helps when you can draw parallels between the abstract software object relationships and real-world relationships, sometimes it makes more sense to have objects that are simply abstractions. For example, you have the concept of an AtBatResult. It seems to me that it would make more sense to have a class named TurnAtBat be responsible for generating the AtBatResult. You could have code like this:

This seems like plausible code and it actually tells a small story. When I design, I try to write the code that I'd like to see first. Then I start writing tests to drive my design thinking down that road and see where it leads me and what kind of concepts present themselves. When the "story" starts becoming confused, that's when I start looking for smells and opportunities to refactor for clarity.

Note that the above may not be the code I end up with when I'm done but it's a starting point for exploring different ways of telling a story. I actually spend more time reading and re-reading my code than I do writing it. Also, I don't try to tell the whole story all at once. I start from a core idea, then work my way outward, adding more details, reading and re-reading to make sure what I have so far still hangs together cohesively and coherently.




Where is the data being passed to the methods in this code? You have to pass the team information somewhere or am I missing something? The atBatResult method has to operate on a player who has to come from a team.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Seems to me that a more natural data structure to represent the bases would be a queue (FIFO). If you see the bases, include home, as spots in a 4-item queue, it seems it's easier to just advance each item (a Player) along in that queue as a result of whatever play is simulated. If a Player gets to the end of the queue without getting tagged out, he scores. If a Player is tagged out at one of the bases, then he gets removed from the queue. You also need to track the players who are back in the batting lineup, which I suppose would have to be some kind of revolving queue. The player who is at the head of the queue goes up to bat and is either put into the bases queue or cycled back to the end of the batting lineup queue. When a Player scores and gets out of the bases queue, he will need to be reinserted in his proper spot in the batting lineup queue.

This might also help simplify the logic for simulating bases being stolen, double plays, triple plays, grand slams, etc.



I didn't think of using a queue because I didn't even know Java had a queue structure when I first wrote this and I didn't think of changing it. I will look at the queue. Thanks! The disadvantage of my last professional coding being in 1986 is when I hear of a FIFO I think of doing it in assembly language which doesn't seem simple at all!
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Kendall Ponder wrote:4. I don't understand what you mean by no tests. Are you talking about debugging?


Besides looking at programs as "stories", I also see them as "theories" of how the computer should do things. To validate these theories, I write tests. The tests will tell me if my theory is flawed or not. For example, one "theory" in this program might be stated as:

1. If the bases are loaded and the player at bat hits a single and the player on third doesn't get tagged out at home, then the team at bat should get another point.

By writing a test, I can start laying out the code that I'd like to write and start to see some of the relationships and collaborations that need to be in place for this story to get played out in the program and my theory of how it should work can be checked.

This approach is called Test-Driven Development and it seems like a "bass ackwards" approach when you first try it but I have found that it really helps clarify and organize my designs.

Take your story, for example:

Line 1 itself is very procedural code. Where does outsInTheInning reside? What class is responsible for tracking this? Where did the number 3 come from? What if the reader doesn't know how many outs there are in an inning? How can this be made more OO? What "theory" is involved here?

Well, the "theory" might be this: "A team will be at bat until it has 3 outs against it" -- Then I would think, "Ok, what classes are involved in telling this in the program and what would the code look like? How do I know that the code is working the way I expect it to work?" Well, I might try this test:

A number of questions will come to mind just by reading this code and it would lead to more design decisions. Refining and clarifying the "story" and "theories" using tests allows me to develop incrementally and iteratively and it helps me to get parts of the program working right away. By seeing working code, I learn more about the problem and how the design fits and whether it's something that can be extended and expanded.

Tell me, with all the code that you've written so far, how many times have you actually run your program? When I'm doing TDD and I have as much code as you have there, I would have executed the tests and the code dozens of times already.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
That test with a TurnAtBat and recording three outs would lead me to write some code that simulates a small scenario: You set up a game, first team goes to bat, and they get three strikes in a row. You can simulate a whole game with just this functionality. The game wouldn't be very much fun to watch because each team will not score at all and the game would come to nine innings with a score of 0-0. But then you learn a lot from this simple simulation. You can then extend it by introducing a result besides OUT, maybe a SINGLE run in the first inning. Then all outs to the end of the game.

You don't have to get everything right all at once. If you try to do that, you will likely fail. It's just way too much to process all at once. You have to step small things in one at a time, always keeping your code base clean and well factored every step of the way. When you extend your story, you re-read the parts that you changed and make sure everything around it still hangs together nice and tight. This is the best way I know to achieve clean code and designs.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Kendall Ponder wrote:Where is the data being passed to the methods in this code? You have to pass the team information somewhere or am I missing something?


Certainly valid questions. This is precisely the point of writing code like this: to elicit these kinds of questions and drive your design thinking. It's about figuring out who's responsible for what and who needs to collaborate with whom. It's about seeing code that will compel you to make decisions that will enable you to answer these questions or write/modify code so that the questions change altogether.

The atBatResult method has to operate on a player who has to come from a team.


Why? The result is just a simulation that's randomly generated, right? Why does a "Player" need to be involved? This is the pitfall I mentioned earlier: you're trying to model the real-world too closely. The questions I would ask instead are:

1. Who needs to know about the play? The Player? The Game? The Inning? The TurnAtBat? Why?
2. How does knowledge of the play affect the state of the object(s) that is/are informed about it?
3. What exactly are these state changes?
4. How do I test that these state changes have occurred?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Other concepts I would explore/play around with:

1. What if I had an OffensivePlay? Examples of this might be: SINGLE, DOUBLE, GRAND_SLAM, BUNT, STEAL, FOUL, LINE_DRIVE, IN_FIELD_FLY, OUT_FIELD_FLY, GROUNDER, HOME_RUN
1a. Are SINGLE, DOUBLE, GRAND_SLAM really offensive plays or something else? Do they belong together with BUNT, STEAL, LINE_DRIVE, FOUL, GROUNDER, etc?

2. What if I had a DefensivePlay? Examples of this might be: STRIKE, BALL, OUT, DOUBLE_PLAY, WALK

3. Would something like a Pitch make more sense? STRIKE, BALL

You can't really get answers just by thinking about these things. You have to come up with some scenarios, write code and tests, and run them and see what the program actually looks like and how it reads.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
One of the "rules" of TDD is to write as little code as possible. You write a little test code, then write a little production code. Rinse and repeat. The first test I might write could be as little as this:

The next rule is to always see a test fail before writing production code to make it pass, so I would write this much production code first:

This will make the test fail. To make the test pass, I make one small change in the production code:

I know this seems silly and a waste of time but it's a stepwise refinement that allows me to actually run the code. It's very important to quickly have code that actually runs so you can start getting feedback about what you've written as soon as possible. Having tests gives you feedback. Without feedback, you're basically shooting in the dark and hoping that you somehow hit your mark.

I then use another test to show that something is wrong. Note that I don't use my intuition that something in the code is wrong but rather I use a test to show that something is wrong.

The 1972 A.M.Turing Award winner, E.W. Dijkstra, said "Program testing can at best show the presence of errors but never their absence." This is the mindset I have when I do this kind of thing.


The code so far will fail this test so I know I need to do something to fix it. So I make my next refinement:

With this small change, both my tests will pass now. This is not much of a program but I have a couple of theories already tested and laid out and I have some code that actually runs. Now I just keep adding new tests to relate additional "theories" and show me that there are still things wrong with the program. These new failing tests will keep compelling me to write more code or modify the existing code to make them pass. At every step of the way, I read and re-read my code and look over my design and make sure that it's all clean.

As little code as this code may be, I can already write a main() method in the Game class that's pretty much what it should be in the final product. It's not much but it's a great start that I can use as a solid foundation on which to build more functionality.

This is exactly the kind of code we recommend you have in main(): see our "Main Is A Pain" article.

Does this kind of give you a glimpse at how it can lead you to good designs and clean code?

I'll leave you with one more quote, this one from a guy named Jack Reeves: "Programming is not about building software; programming is about designing software."
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Kendall Ponder wrote:4. I don't understand what you mean by no tests. Are you talking about debugging?


Besides looking at programs as "stories", I also see them as "theories" of how the computer should do things. To validate these theories, I write tests. The tests will tell me if my theory is flawed or not. For example, one "theory" in this program might be stated as:

1. If the bases are loaded and the player at bat hits a single and the player on third doesn't get tagged out at home, then the team at bat should get another point.

By writing a test, I can start laying out the code that I'd like to write and start to see some of the relationships and collaborations that need to be in place for this story to get played out in the program and my theory of how it should work can be checked.

This approach is called Test-Driven Development and it seems like a "bass ackwards" approach when you first try it but I have found that it really helps clarify and organize my designs.

Take your story, for example:

Line 1 itself is very procedural code. Where does outsInTheInning reside? What class is responsible for tracking this? Where did the number 3 come from? What if the reader doesn't know how many outs there are in an inning? How can this be made more OO? What "theory" is involved here?

Well, the "theory" might be this: "A team will be at bat until it has 3 outs against it" -- Then I would think, "Ok, what classes are involved in telling this in the program and what would the code look like? How do I know that the code is working the way I expect it to work?" Well, I might try this test:

A number of questions will come to mind just by reading this code and it would lead to more design decisions. Refining and clarifying the "story" and "theories" using tests allows me to develop incrementally and iteratively and it helps me to get parts of the program working right away. By seeing working code, I learn more about the problem and how the design fits and whether it's something that can be extended and expanded.

Tell me, with all the code that you've written so far, how many times have you actually run your program? When I'm doing TDD and I have as much code as you have there, I would have executed the tests and the code dozens of times already.



I had the entire program debugged and working before I learned about clean code and started to clean it up. I did all of the start small and test each part (although when I had a menu I had stub methods and then would build and test each of them one at a time). I didn't see the point of including test code here, I was looking for suggestions on the structure of the code. I agree with your program development description but I have already done all of that. I used Eclipse and tested every scenario and made numerous changes to my original logic in the process.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Kendall Ponder wrote:Where is the data being passed to the methods in this code? You have to pass the team information somewhere or am I missing something?


Certainly valid questions. This is precisely the point of writing code like this: to elicit these kinds of questions and drive your design thinking. It's about figuring out who's responsible for what and who needs to collaborate with whom. It's about seeing code that will compel you to make decisions that will enable you to answer these questions or write/modify code so that the questions change altogether.

The atBatResult method has to operate on a player who has to come from a team.


Why? The result is just a simulation that's randomly generated, right? Why does a "Player" need to be involved? This is the pitfall I mentioned earlier: you're trying to model the real-world too closely. The questions I would ask instead are:

1. Who needs to know about the play? The Player? The Game? The Inning? The TurnAtBat? Why?
2. How does knowledge of the play affect the state of the object(s) that is/are informed about it?
3. What exactly are these state changes?
4. How do I test that these state changes have occurred?


The information contained in the Player object has to be used in any AtBat method (note: in my experience the phrase TurnAtBat is often used for a teams turn at bat so I find that name confusing because it could mean the same as an inning). I have asked all of these questions as I went through the design and debugging process and decided to have Player and Team objects. It isn't clear to me what problems those decisions cause that I need to fix. I am open to a different way of storing and using the data I have in my Player object. I'm not sure why you think I haven't asked all of these questions. Nothing in the code could tell you whether I spent 5 minutes knocking something out, are 5 weeks going back and forth on the best way to do it.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:That test with a TurnAtBat and recording three outs would lead me to write some code that simulates a small scenario: You set up a game, first team goes to bat, and they get three strikes in a row. You can simulate a whole game with just this functionality. The game wouldn't be very much fun to watch because each team will not score at all and the game would come to nine innings with a score of 0-0. But then you learn a lot from this simple simulation. You can then extend it by introducing a result besides OUT, maybe a SINGLE run in the first inning. Then all outs to the end of the game.

You don't have to get everything right all at once. If you try to do that, you will likely fail. It's just way too much to process all at once. You have to step small things in one at a time, always keeping your code base clean and well factored every step of the way. When you extend your story, you re-read the parts that you changed and make sure everything around it still hangs together nice and tight. This is the best way I know to achieve clean code and designs.



I did all of this. Why do you think I didn't? Robert Martin in his Clean Code book says refactoring is a continual process and his first attempt is never well factored. Mine definetly was not well factored since I had never heard of the clean code idea. I am trying to clean it up now.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Kendall Ponder wrote:

Junilu Lacar wrote:Seems to me that a more natural data structure to represent the bases would be a queue (FIFO). If you see the bases, include home, as spots in a 4-item queue, it seems it's easier to just advance each item (a Player) along in that queue as a result of whatever play is simulated. If a Player gets to the end of the queue without getting tagged out, he scores. If a Player is tagged out at one of the bases, then he gets removed from the queue. You also need to track the players who are back in the batting lineup, which I suppose would have to be some kind of revolving queue. The player who is at the head of the queue goes up to bat and is either put into the bases queue or cycled back to the end of the batting lineup queue. When a Player scores and gets out of the bases queue, he will need to be reinserted in his proper spot in the batting lineup queue.

This might also help simplify the logic for simulating bases being stolen, double plays, triple plays, grand slams, etc.



I didn't think of using a queue because I didn't even know Java had a queue structure when I first wrote this and I didn't think of changing it. I will look at the queue. Thanks! The disadvantage of my last professional coding being in 1986 is when I hear of a FIFO I think of doing it in assembly language which doesn't seem simple at all!



I have been thinking about using a queue and I don't see how it helps because everything on the base queue doesn't move one at a time. If a walk pushes a man on the base queue that doesn't automatically push everyone else on the bases so I have to have logic to test for that which seems to defeat the purpose of using the queue unless a FIFO queue in Java means something different than it did when I used them in my assembly language programs.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Kendall Ponder wrote:
I did all of this. Why do you think I didn't? Robert Martin in his Clean Code book says refactoring is a continual process and his first attempt is never well factored. Mine definetly was not well factored since I had never heard of the clean code idea. I am trying to clean it up now.


The nuance is that Uncle Bob refactors after writing a few lines of code and in between running tests every couple of minutes. Literally, every two minutes or so. The way your code is right now, there are way too many smells and that hints at a lack of refactoring along the way. Not that this code can't be refactored but it's just going to be more challenging to reverse the effects of some of the poor design choices.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Regarding the queue, I don't think you'll be able to use a standard queue but rather use the concept of a queue as the basis for tracking movement around the bases. It's just an idea / theory at this point. I would have to write tests and code to test the theory and discover how well it fits the problem.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
It's hard to discern emotions in a medium like this and I've tried my best to be as clinical and impersonal with the critiques I have given. Please don't take any of what I say personally because it is never my intent to do anything other than help. It's always about the code, never about the coder.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When I review code, I always look at the tests first if there are any. They usually give an insight into the thought processes that went into writing the production code. If you buy into Jack Reeves' proposition that code is the design, then the tests are the detailed design specifications. So it would actually help if you showed some of the tests you have that exercise some the code that you feel needs to be cleaned up.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
An inning is when each team takes a turn at bat. At the top of the inning the visitor is at bat. At the bottom of the inning, the home team bats. Then you start the next inning. I'll have to look again to see if you have this idea codified in the program or if you even need to (seems to me that it should though). The real question is "who is responsible for generating the random play?" It may not even be a TurnAtBat object. Again that is just a theory I wanted to try out to see what alternatives there might be to having a Player generate a random play.
 
Kendall Ponder
Ranch Hand
Posts: 205
4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:It's hard to discern emotions in a medium like this and I've tried my best to be as clinical and impersonal with the critiques I have given. Please don't take any of what I say personally because it is never my intent to do anything other than help. It's always about the code, never about the coder.



I agree it is hard to discern emotions in this medium and I tried not to sound offended (I'm not) when I asked why you thought I didn't do tests, I really wanted to know what about the code made you think that because that would help me understand how to improve the code so it looked like I did do tests. I think the best way to learn is to be critiqued and to question the critique until you totally understand it. I appreciate the amount of time you (and others) have spent looking at the code but I will never be able to apply the suggestions everyone is making on future code unless I really see how the suggestions improve the code and solve the issues I was trying to solve in the code.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
The issue I had on seeing the play generated by a Player is that the "player" concept isn't just about who is at bat. There are players who are on the bases, running. There are players in the dugout, waiting for their turn to bat. To have Player generate a random play like DOUBLE, SINGLE, etc. doesn't seem to fit. This is why it would be helpful to see what tests you've written. Tests should provide succinct examples of how the code should be used. If I have to look at the code, there's a lot more I have to sift through. At any rate, I see this code:

All this code is in the Team class. The ideas don't fit very well together though. Does a team update runs in the inning? Does a team advance runners? Who is responsible for tracking where the runners are? Should it be the team? Should there be a Diamond object instead? The idea/theory might be that a Diamond has Bases and it tracks which ones have runners on them.

Let's look at the updateRunsInTheInning() method for clues. This method is awfully busy. There's a lot going on in that switch-case statement. Shouldn't runs only be updated when a runner safely makes it home? And there are rules about recording runs, too, like whether a run counts or not on a third out. Are you going to make that part of the simulation? At the very least, this method needs to be broken apart so that a team's score is only updated when a runner makes it home safely. Here's some code that you might try to write around:


The question with this code would be where would it reside? In a Diamond object? Does that make sense? What are the interactions that would need to happen between Diamond, Base, and Runner for this code to work? Again, you won't know for sure until you try to write it down and see it running.

Edit: I almost forgot the most important question: How do I test this?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Looking at the code

runner.advanceOn(play);

it seems that a Play would have to help determine how many bases to advance. So, SINGLE.advanceBy() would return 1. Another way to think about it might be SINGLE.advanceFrom(FIRST) would return SECOND and DOUBLE.advanceFrom(SECOND) would return HOME and DOUBLE.advanceFrom(THIRD) still returns HOME. How might this look in code?


This code seems reasonably clean. Can I run with it? What other tests can I write to exercise it? Where do I put these methods? In a Runner class? Or in the Player class? Again, I would try these options out and see what makes sense.

Then you ask, well how does a player/runner get on base? Would you have an AT_BAT base? Would that mean that the Play enum would be the most sensible place to put a method that randomly generates a play? What a revolutionary concept, right?


All the above code makes having a Player object seem more justifiable since this behavior makes sense to have in a Player object. Or maybe a Runner object. I don't know. Have to try and see it in the context of more code.

Should we have this code then?
 
This is my favorite tiny ad:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic