I have a golf application and I want to update an Scorecard which has score, nettScore, points each for 18 holes, so 54 in total
I have spent a few days getting to the point I am at now, All works fine, but I feel there must be a more efficient way of doing this. So far I have this, (ent refers to an Entrant and e refers to an event)
public void update(long eventId, long memberId, ScoreCard scorecard) {
Event e = eventRepo.findEventById(eventId);
Member m = memberRepo.findMemberById(memberId);
double courseHandicapDouble = m.getHandicap()/113*e.getCourse().getSlopeRating();
int courseHcp = (int) Math.round(courseHandicapDouble);
int stableFordTotal = 0;
int medalTotal = 0;
int nettTotal = 0;
List<Entrants> en = entrantsRepo.findAllByEvent(e);
for(Entrants ent : en) {
if (ent.getMember().equals(m)) {
//Hole 1
//Set the entrants score to the value of hole1 score on the scorecard
ent.getScoreCard().setH1Score(scorecard.getH1Score());
//If the players course handicap is more than or equal to the stroke index for the hole, set nett score to score -1
if(courseHcp >= e.getCourse().getHoles().get(0).getStrokeIndex()) {
ent.getScoreCard().setH1NettScore(scorecard.getH1Score() - 1);
//Set points for the first hole as per the setPoints helper method.
ent.getScoreCard().setH1Points(setPointsPar(ent.getScoreCard().getH1NettScore(), e.getCourse().getHoles().get(0).getPar()));
} else {
ent.getScoreCard().setH1NettScore(scorecard.getH1Score());
ent.getScoreCard().setH1Points(setPointsPar(ent.getScoreCard().getH1NettScore(), e.getCourse().getHoles().get(0).getPar()));
}
//Update running total scores
medalTotal = ent.getScoreCard().getH1Score();
stableFordTotal = ent.getScoreCard().getH1Points();
nettTotal = ent.getScoreCard().getH1NettScore();
//repeated 18 times for holes 2-18
The helper method to work out the points is a simple switch method that gives points depending on the score and the par of the hole.
public int setPointsPar(int score, int par){
int points = 0;
if(par == 3) {
switch (score) {
case 1:
points = 4;
break;
case 2:
points = 3;
break;
case 3:
points = 2;
break;
case 4:
points = 1;
break;
}
}
if(par == 4) {
switch (score) {
case 1:
points = 5;
break;
case 2:
points = 4;
break;
case 3:
points = 3;
break;
case 4:
points = 2;
break;
case 5:
points = 1;
break;
}
}
if(par == 5) {
switch (score) {
case 1:
points = 6;
break;
case 2:
points = 5;
break;
case 3:
points = 4;
break;
case 4:
points = 3;
break;
case 5:
points = 2;
break;
case 6:
points = 1;
break;
}
}
return points;
}
This part of the code is almost duplicated 18 times, the only difference being that h1Score is change to h2Score and h1NettScore to h2NettScore etc
//Hole 1
//Set the entrants score to the value of hole1 score on the scorecard
ent.getScoreCard().setH1Score(scorecard.getH1Score());
//If the players course handicap is more than or equal to the stroke index for the hole, set nett score to score -1
if(courseHcp >= e.getCourse().getHoles().get(0).getStrokeIndex()) {
ent.getScoreCard().setH1NettScore(scorecard.getH1Score() - 1);
//Set points for the first hole as per the setPoints helper method.
ent.getScoreCard().setH1Points(setPointsPar(ent.getScoreCard().getH1NettScore(), e.getCourse().getHoles().get(0).getPar()));
} else {
ent.getScoreCard().setH1NettScore(scorecard.getH1Score());
ent.getScoreCard().setH1Points(setPointsPar(ent.getScoreCard().getH1NettScore(), e.getCourse().getHoles().get(0).getPar()));
}
//Update running total scores
medalTotal = ent.getScoreCard().getH1Score();
stableFordTotal = ent.getScoreCard().getH1Points();
nettTotal = ent.getScoreCard().getH1NettScore();
I can't think of any possible way to loop through the above so that all the 1s are changed to 2s, then 3s etc.
As I say, what I have works fine. It is more of a learning point for me as I like to write as efficiently as possible.
CodePudding user response：
First you can calculate the score as simply par 2 - score and get rid of the case statements.
Second, your ScoreCard class should have a number of Map<Integer,Integer>s which contain the score, points and nett score for each hole, so you can write:
void updateForHole(Event e, Entrants ent, int hole) {
//Set the entrants score to the value of hole1 score on the scorecard
ent.getScoreCard().setScore(hole, scorecard.getScore(hole));
//If the players course handicap is more than or equal to the stroke index for the hole, set nett score to score -1
if(courseHcp >= e.getCourse().getHoles().get(hole).getStrokeIndex()) {
ent.getScoreCard().setNettScore(hole, scorecard.getScore(hole) - 1);
//Set points for the first hole as per the setPoints helper method.
ent.getScoreCard().setPoints(hole, setPointsPar(ent.getScoreCard().getNettScore(hole), e.getCourse().getHoles().get(hole).getPar()));
} else {
ent.getScoreCard().setNettScore(hole, scorecard.getScore(hole));
ent.getScoreCard().setPoints(hole, setPointsPar(ent.getScoreCard().getNettScore(hole), e.getCourse().getHoles().get(0).getPar()));
}
//Update running total scores
medalTotal = ent.getScoreCard().getScore(hole);
stableFordTotal = ent.getScoreCard().getPoints(hole);
nettTotal = ent.getScoreCard().getNettScore(hole);
}
This is similar to the approach you already use for getting a particular hole from the course.
Then you call updateForHole in a loop: (I'm using zero for hole 1 to match what you've done with the Course already)
for (int hole = 0; hole < e.getCourse().getHoles().size(); hole ) {
updateForHole(hole);
}
The get and set functions could look like this:
class ScoreCard {
private final Map<Integer,Integer> scores = new HashMap<>();
...
public void setScore(int hole, int score) {
scores.put(hole, score);
}
public int getScore(int hole) {
return scores.get(hole);
}
...
}
If you call getScore for a hole which you haven't set a score for yet, it will throw a NullPointerException, because the value in the Map for that hole will be null. That will tell you that you have a logic bug in your program, and you are trying to use a value before you've set it. Finding that out is usually a good thing.
CodePudding user response：
Instead of 54 different fields, Scorecard should have a list of 18 Hole objects. The Hole instances should contain the required per-hole fields. I would suppose those to be something like par and strokes. The points would be a function of those values, perhaps something like Math.max(par - strokes 2, 0)
You'd then loop over the list of Hole instances to compute the overall scores for the game, without repeating any code.