Dave Tolls wrote:Junilu, out of curiosity, in your Play enum above, why have an abstract gain() method rather than a concrete one that returns a given gain attribute, populated in an enum constructor?
There seems to be a lot of code duplication there with all the Overrides of gain().
Of course, I might have missed something...it has been known.
Junilu Lacar wrote: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. ?
Junilu Lacar wrote:Hi Kendall,
Yeah, I had an inkling that statistics were going to be involved at some point here. That might make having a method in Player that gives back SINGLE, DOUBLE, etc. appropriate but I still think the name atBat itself is not a good one. There's probably a name that will fit better and make you go "Aha! Yes, that's what I meant to say!"
Junilu Lacar wrote:Tim Ottinger actually wrote the chapter about Names in CC and I think he also has some collaborations with Jeff Langr, "Agile in a Flash" if I'm not mistaken.
Kendall Ponder wrote: I think one reason names are so difficult is the namer knows exactly what the program does and knows the point they intend to get across. I know your name likelyResult intends to convey the fact that the result is related to the probability of the player having a particular result. But I think if I was reading this the first time I would think it was giving a likely result based on the players stats but it may actually be an unlikely result. ... it is difficult to predict the meaning a particular reader will put on a name.
Junilu Lacar wrote:
Kendall Ponder wrote:
So to get a statistically influenced "at bat result", you'd have to pick a play at random and yet skew the result in favor of past performance trends while still leaving the possibility of getting an "unlikely" play. The only thing I can think of is to set up an array of 100 plays with a distribution of plays based on the player's statistics. You would then shuffle these choices and pick one out at random. This responsibility still seems to be best given to the Statistics object to handle but I can't think of a good name for it. I'm going to go with randomPlay() for now. Sometimes when my team and I are stuck on a name like this, we just use a nonsense one like foo() or blah() and that sometimes forces our brains to work harder to find the right name. YMMV.
What I did was find the probability of all possible outcomes and made a series of cut-offs and compared a random number to them. For example if a player has a 20% of getting a hit, 30% of a walk and 50% of an out the hit cut-off would be 0.2 and the walk cut-off would be 0.2+0.3=0.5. Then if the random number is less than 0.2 it is a hit, else less than 0.5 a walk, else an out. I picked some player stats from the MLB site to find reasonable percentages. When I tested 100,000 trials the distribution matched the probability. I don't have the test code because I didn't know about unit testing at the time.
Junilu Lacar wrote:Tim Ottinger actually wrote the chapter about Names in CC and I think he also has some collaborations with Jeff Langr, "Agile in a Flash" if I'm not mistaken. Anyway, I have a feeling that the Law of Demeter will be important to keep in mind when you program around the collaboration between Player and Statistics. Corey Haines' book has some interesting examples of how applying LoD to reverse perspectives can result in cleaner code. So instead of using a bunch of getter methods to gather values and do calculations, you might have the Player pass itself to a Statistics object:
Since the stats object is the "information expert" on the percentages, it's best to keep calculations that involve percentages encapsulated within the Stats class. In other words, you tell it to calculate likely outcomes rather than ask it for information so that you can calculate likely outcomes. Does that make sense?
Sandy Metz wrote:Don't write code that guesses the future, arrange code so you can adapt to the future when it arrives.
Kendall Ponder wrote:Is it better to have the Player class only consist of fields holding a player's statistics and the corresponding getters and setters and have a separate Statistics class which uses a Player object to generate a result for a single player's at bat or just have a Player object which has one public method (other than getters and setters) which generates a result for a single players at bat? I think the Law of Demeter will not be violated in either case. The logic of the code will be the same in either case but in the first case you will have to use getters to access all of the Player data which I think makes the code a little harder to read, but maybe it doesn't for more experienced Java programmers. Does having the Player class hold the player data and have a method which generates an at bat result violate SRP? A related question (at least I think it is related) is there a preferred way to write this program to allow for future modification? What if I want to add new stats to a player? I know I am going to think of new ways to compare and present scenarios. Is writing clean code the main thing to allow for modification? Are there design patterns which would help? Thanks!
Kendall Ponder wrote:I appreciate the amount of time you (and others) have spent looking at the code...