A more efficient way of updating object field values in Java

Question

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.

Answer 1

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.

Answer 2

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.