Боулинг-бомбардир

Мне было предоставлено простое упражнение по кодированию в рамках процесса собеседования, для которого я получил отрицательный отзыв.

Это вопрос :

  

DiUS начинает боулинг-клуб. Чтобы помочь с клубом, мы помогли вам запрограммировать систему подсчета очков.

     

В системе есть следующие функции:

     
  • Только один игрок
  •   
  • В каждом кадре котелок имеет 2 попытки сбить все контакты
  •   
  • Если в 2 попытках котелок не сбивает все контакты, их оценка - это сумма количества выводов, которые они сбили в двух попытках   
    • Например, если котелок работает, 4,4, то их оценка равна 8.
    •   
  •   
  • Если в 2 попытках котелок сбивает все штифты, он является запасным. Скоринг запасной - это сумма количества сбитых штифтов плюс количество штифтов, сбитых в следующей чаше.

         
    • Например, если котелок работает, 4,6 | 5,0, тогда их оценка равна 20 = (4 + 6 + 5) + (5 + 0).
    •   
  •   
  • Если в одной попытке котелок сбивает все штифты, это удар. Забивание забастовки - это сумма количества сбитых штифтов плюс количество штифтов, сбитых в следующих двух чашах.

         
    • Например, если котелок валяется, 10 | 5, 4, тогда их оценка равна 28 = (10 + 5 + 4) + (5 + 4).
    •   
  •   
  • В кадре есть 10 контактов
  •   
  • Есть 10 кадров в матче
  •   
  • Не беспокойтесь о проверке количества рулонов в кадре
  •   

Интерфейс должен выглядеть так (в Java);

bowlingGame.roll(noOfPins);
bowlingGame.score();

Вот мое решение:

MatchFactory.java

public final class MatchFactory {

    private MatchFactory(){};

    public static BowlingGame createMatch() {
        return new BowlingGameScoreBoard();
    }

}

BowlingGame.java

public interface BowlingGame {

    /**
     * Keeps track of pins knocked over
     * @param noOfPins knocked over per frame
     * @exception {@link au.com.dius.BowlingGameScoreBoard.BowlingException}
     */
    public void roll(int noOfPins);

    /**
     *
     * @return score of current frame only
     */
    public int score();
}

BowlingGameScoreBoard.java

public final class BowlingGameScoreBoard implements BowlingGame {


        private final List<Frame> frames;
        private static final int MAX_FRAMES = 10;
        private static final int MAX_PINS = 10;
        private static final int MAX_ATTEMPTS_PER_FRAME = 2;
        private int frameCounter = 0;
        private int strikeCounter = 0;
        private static final int ALL_STRIKE_SCORE = 300;


        /**
         * setup the game, ie all {@link BowlingGameScoreBoard#MAX_FRAMES} frames
         */
        public BowlingGameScoreBoard() {

            frames = new ArrayList<Frame>(MAX_FRAMES);

            for (int i = 0; i < MAX_FRAMES; i++) {
                frames.add(new Frame());
            }
        }

        @Override
        public void roll(int noOfPins) {

            if (noOfPins > MAX_PINS) {
                throw new BowlingException("illegal argument " + noOfPins);
            }

            Frame frame = getFrame();

            if (frame == null) {
                throw new BowlingException("all attempts exhausted - start new game");
            }

            frame.setScore(noOfPins);

            if (isBonusFrame()) {
                Frame prev = getPreviousFrame();
                // restrict to one attempt, when last frame was spare
                if (prev.isSpare()) {
                    frame.limitToOneAttempt();
                }
            }

        }

        /**
         * returns a frame and moves to next frame if current has used up attempts
         * @return {@link au.com.dius.BowlingGameScoreBoard.Frame}
         */
        private Frame getFrame(){

            Frame frame = getCurrentFrame();

            if (frame.isDone()) {

                // new bonus frame
                if(isLastFrame() && (frame.isSpare() || frame.isStrike())) {
                    Frame bonus = new Frame();
                    frames.add(bonus);
                    frameCounter++;
                    return bonus;
                }

                frameCounter++;
                if (frameCounter == MAX_FRAMES || isBonusFrame()) {
                    return null;
                }

                frame = getCurrentFrame();
            }

            return frame;
        }


    @Override
    public int score() {

        int score;

        // first frame
        if (frameCounter == 0) {

            Frame curr = getCurrentFrame();
            return curr.score();

        } else {

            // score 300, strikes for all frames
            if (isLastFrame() && isAllStrikes()) {
                return ALL_STRIKE_SCORE;
            }

            Frame curr = getCurrentFrame();
            Frame prev = getPreviousFrame();

            // only add previous last frame to current score
            if (isBonusFrame()) {
                return prev.score() + curr.score();
            }

            score = curr.score();

            if(prev.isSpare()) {
                score += (prev.score() + curr.getFirstScore());
            }

            if(prev.isStrike()) {
                score += (prev.score() + curr.getFirstScore() +  curr.getSecondScore());
            }

        }

        return score;
    }

    private Frame getPreviousFrame() {
        return frames.get(frameCounter-1);
    }

    private Frame getCurrentFrame() {
        return frames.get(frameCounter);
    }

    private boolean isAllStrikes() {
    return strikeCounter == MAX_FRAMES ;
}

    private boolean isBonusFrame() {
        return frames.size() > MAX_FRAMES;
    }

    private boolean isLastFrame() {
        return frameCounter == MAX_FRAMES - 1;
    }

    /**
     * This nested class encapsulates the concept of a frame
     * and manages score and attempts allowed for each frame
     */
    private class Frame {

        private int[] scores = new int[MAX_ATTEMPTS_PER_FRAME];
        private int noOfPins = 10;
        private int noAttempts = 0;
        private boolean isStrike = false;

        private boolean isSpare() {
            return noOfPins == 0 && noAttempts == MAX_ATTEMPTS_PER_FRAME && !isStrike;
        }

        private boolean isStrike() {
            return noOfPins == 0 && noAttempts == MAX_ATTEMPTS_PER_FRAME && isStrike;
        }

        private boolean isDone () {
            return noAttempts == MAX_ATTEMPTS_PER_FRAME;
        }

        private void setScore(int score) {

            scores[noAttempts++] = score;
            noOfPins -= score; // keep track of remaining pins/frame

            if (score == MAX_PINS) {
                isStrike = true;
                strikeCounter++;
            }
        }

        private void limitToOneAttempt(){
            scores[1] = 0;
            noAttempts++;
        }

        private int score() { return scores[0] + scores[1];}

        private int getFirstScore() {
            return scores[0];
        }

        private int getSecondScore() {
            return scores[1];
        }

    }

    /**
     * Represents an exception for the bowling game
     */
    public class BowlingException extends RuntimeException {

        BowlingException(String message) {
            super(message);
        }

    }
}

BowlingGameTest.java

@RunWith(JUnit4.class)
public class BowlingGameTest {


    private BowlingGame game;
    private static final int MAX_ATTEMPTS = 20;

    @Before
    public void setUp() {
        game = MatchFactory.createMatch();
    }


    @Test
    public void testScoreNoSpareOrStrike() {

        game.roll(4);
        game.roll(4);

        int score  = game.score();
        Assert.assertEquals(8, score);

    }

    @Test
    public void testSpare() {

        game.roll(4);
        game.roll(6);

        int score  = game.score();
        Assert.assertEquals(10, score);

        game.roll(5);
        game.roll(0);

        score  = game.score();
        Assert.assertEquals(20, score);

    }


    @Test
    public void testStrikeOnSecondAttempt() {

        game.roll(0);
        game.roll(10);

        int score  = game.score();
        Assert.assertEquals(10, score);


        game.roll(5);
        game.roll(4);

        score  = game.score();
        Assert.assertEquals(28, score);

    }


    @Test
    public void testStrikeOnFirstAttempt() {

        game.roll(10);
        game.roll(0);

        int score  = game.score();
        Assert.assertEquals(10, score);


        game.roll(5);
        game.roll(5);

        score  = game.score();
        Assert.assertEquals(30, score);

    }


    @Test
    public void testStrikeEveryRoll() {

        for (int i = 0; i < 10 ; i++) {

            game.roll(10);
            game.roll(0);
        }

        int score = game.score();
        Assert.assertEquals(300, score);

    }


    @Test
    public void testLastFrameSpare() {

        for (int i = 0; i < 10 ; i++) {

            game.roll(5);
            game.roll(5);
        }

        game.roll(5);

        int score = game.score();
        Assert.assertEquals(15, score);
    }

    @Test
    public void testLastFrameStrike() {

        for (int i = 0; i < 10 ; i++) {

            game.roll(10);
            game.roll(0);
        }

        game.roll(3);
        game.roll(4);

        int score = game.score();
        Assert.assertEquals(17, score);
    }


    @Test(expected = BowlingGameScoreBoard.BowlingException.class)
    public void testLastFrameNoStrike() {

        for (int i = 0; i < 10 ; i++) {

            game.roll(3);
            game.roll(5);
        }
        // this wont happen as last frame wasnt strike/spare
        game.roll(3);
        game.roll(4);

    }

    /**
     * Exception is generated if try and go beyond 10 frames / match
     */
    @Test(expected = BowlingGameScoreBoard.BowlingException.class)
    public void testPlayMoreThanAllFrames() {

        for (int i = 0; i <= MAX_ATTEMPTS  ; i++) {
            game.roll(i/10);
        }
    }


    /**
     * This tests an illegal argument , ie cant pass more than 10 pins to
     * knock down
     *
     * I'am using custom exception here instead of Java's {@link java.lang.IllegalArgumentException}
     */
    @Test(expected = BowlingGameScoreBoard.BowlingException.class)
    public void testIllegalBowlException() {

            game.roll(200);

    }


}

Я получил сообщение:

  

Pros

     
  • Имеет тест
  •   
  • Разбитое решение на 2 класса (игра в боулинг /рамка)
  •   

против

     
  • Не рулон снова, если это удар
  •   
  • MatchFactory кажется ненужным
  •   
  • Метод подсчета немного сложнее.
  •   
  • Не работает сценарий с несколькими ударами
  •   
  • Не знаю, почему Frame является частным классом
  •   

Я хотел бы получить обзор и соглашения /разногласия с отзывами.

11 голосов | спросил user50772 9 AM00000080000003631 2014, 08:52:36

2 ответа


5

Я согласен с большинством отзывов. Не должно быть рулона после удара в раме. Рамка содержит только бросок удара.

Обычно рекомендуется «скрыть» реализации интерфейса с фабрикой, чтобы пользователи интерфейса не зависели от конкретных классов. Но в этом случае наличие фабрики слишком велико, так как существует только один класс, реализующий интерфейс, и только один его класс (BowlingGameTest) , Вы даже можете избавиться от интерфейса.

В этом вопросе явно говорится:

  

Не беспокойтесь о проверке количества рулонов в кадре

В упражнении предполагается, что вызовы bowlingGame.roll(noOfPins) всегда верны. Так зачем беспокоиться об исключениях и проверках вроде if (noOfPins > MAX_PINS)?

Пробные тесты хороши. :) Имена тестовых методов довольно близки к описанию того, что такое тест, но они могут быть еще лучше. Представьте, что вы сгибаете весь тестовый код и просто читаете имена методов: нужно знать, что должен делать проверенный код, читая именно это. Именам не нужно начинать со слова «тест», мы все-таки в тестовом классе, а аннотация @Test уже говорит Это. В ваших тестах отсутствуют некоторые простые случаи: что произойдет, если игрок перевернет 0 для всей игры? или все запасные части? (есть testStrikeEveryRoll() хотя)

Дополнительные примечания по типу теста:

int score = game.score();
Assert.assertEquals(300, score);

Эта переменная score бесполезна, inline it. Слишком много пустых строк на мой вкус, это заставляет меня прокручивать, чтобы читать все, тем самым ухудшая читаемость. Вы также можете извлечь цикл, который выполняется несколько раз в отдельном методе, чтобы сделать тесты более читабельными.

private void rollMany(int pins, int times) {
  for (int i = 0; i < times; i++) {
    game.roll(pins);
  }
}

Наконец, правила подсчета в score() не выпрыгивают в голове читателя. Хотя этот бит

if(prev.isSpare()) {
  score += (prev.score() + curr.getFirstScore());
}
if(prev.isStrike()) {
  score += (prev.score() + curr.getFirstScore() + curr.getSecondScore());
}

начинает выглядеть как настоящая вещь. Для справки, вот как выглядит метод score() в Бойлинг-игра Роберта Мартина :

public int score() {
  int score = 0;
  int frameIndex = 0;
  for (int frame = 0; frame < 10; frame++) {
    if (isStrike(frameIndex)) {
      score += 10 + strikeBonus(frameIndex);
      frameIndex++;
    } else if (isSpare(frameIndex)) {
      score += 10 + spareBonus(frameIndex);
      frameIndex += 2;
    } else {
      score += sumOfBallsInFrame(frameIndex);
      frameIndex += 2;
    }
  }
  return score;
}

Разве правила здесь не очевидны? ;)

Если вы хотите практиковать это упражнение, я рекомендую http://cyber-dojo.org/. Вы можете выполнять групповые сеансы и сравнивать свои решения. Я делал это со своими коллегами несколько раз, чтобы тренировать TDD, это было весело.

ответил Kolargol00 9 AM000000100000003931 2014, 10:38:39
4

Единичные тесты

В ваших модульных тестах содержатся существенные неточности. К сожалению, ошибка в тестировании ставит под сомнение как тестовый код, так и основную программу.


@Test
public void testStrikeOnSecondAttempt() {

    game.roll(0);
    game.roll(10);

    int score  = game.score();
    Assert.assertEquals(10, score);


    game.roll(5);
    game.roll(4);

    score  = game.score();
    Assert.assertEquals(28, score);

}

Эти ролики на самом деле являются запасными, за ними стоит кадр из 5,4. Счет должен быть \ $ (0 + 10 + \ color {red} {5}) + (5 + 4) = 24 \ $, при этом красный 5 является бонусом для запасного.

Судя по названию теста и утвержденному результату, я бы предположил, что вы, вероятно, хотели прокатить 0,0 | 10 | 5,4. Тогда оценка должна быть \ $ (0 + 0) + (10 + \ color {red} {5 + 4}) + (5 + 4) = 28 \ $.


@Test
public void testStrikeOnFirstAttempt() {

    game.roll(10);
    game.roll(0);

    int score  = game.score();
    Assert.assertEquals(10, score);


    game.roll(5);
    game.roll(5);

    score  = game.score();
    Assert.assertEquals(30, score);

}

Рулоны представляют кадры 10 | 0,5 | 5,?, Где третий кадр является неполным. Оценка для этих рулонов должна быть \ $ (10 + \ color {red} {0 + 5}) + (0 + 5) + 5 = 25 \ $, а не 30.

Возможно, вы хотели бросить 10 | 5,5. Оценка для этого будет: \ $ (10 + \ color {red} {5 + 5}) + (5 + 5 + \ color {red} {?}) = 30 \ $, при этом предстоящий бросок будет удвоен как бонус для запасного во втором кадре.


@Test
public void testStrikeEveryRoll() {

    for (int i = 0; i < 10 ; i++) {

        game.roll(10);
        game.roll(0);
    }

    int score = game.score();
    Assert.assertEquals(300, score);

}

Идеальная игра должна состоять из дюжины рулонов по 10 (десять ударов, плюс два броска только для их бонусов). Не должно быть никаких желобков. То, что вы на самом деле писали, было 10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0. Счет для этого незавершенного будет 110, а котелок не сможет использовать любые бонусы, кроме третьего броска.


@Test
public void testLastFrameSpare() {

    for (int i = 0; i < 10 ; i++) {

        game.roll(5);
        game.roll(5);
    }

    game.roll(5);

    int score = game.score();
    Assert.assertEquals(15, score);
}

Это не имело никакого смысла. Из этих бросков только одни контакты должны быть оценены в 105 ... и бонусы для запасных частей добавят еще 50. Эти ролики также не имеют никакого отношения к названию теста.

Последовательность, которая удовлетворяет как утверждению, так и названию теста, может быть 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,10 | 5.


@Test
public void testLastFrameStrike() {

    for (int i = 0; i < 10 ; i++) {

        game.roll(10);
        game.roll(0);
    }

    game.roll(3);
    game.roll(4);

    int score = game.score();
    Assert.assertEquals(17, score);
}

Это так же бессмысленно, как и предыдущий тест. Последовательность 10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,10 | 0,3 | 4 является незаконным, так как игра закончилась до качки 3.

Возможная последовательность, которая удовлетворяет как утверждению, так и названию теста, может быть 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 0,0 | 10 | 3,4.


@Test(expected = BowlingGameScoreBoard.BowlingException.class)
public void testLastFrameNoStrike() {

    for (int i = 0; i < 10 ; i++) {

        game.roll(3);
        game.roll(5);
    }
    // this wont happen as last frame wasnt strike/spare
    game.roll(3);
    game.roll(4);

}

Этот тест несколько глючит, поскольку он дает ложную уверенность. После десяти кадров, каждый из 3,5, игра закончилась бы. Роллинг 3 должен быть достаточным, чтобы вызвать исключение. Таким образом, запись вашего теста, а также roll 4 слишком мягка, давая ему второй шанс выбросить исключение, которое должно было быть выброшено ранее.


@Test(expected = BowlingGameScoreBoard.BowlingException.class)
public void testPlayMoreThanAllFrames() {

    for (int i = 0; i <= MAX_ATTEMPTS  ; i++) {
        game.roll(i/10);
    }
}

Это не ошибка per se . Однако заголовок цикла немного вводит в заблуждение. Идиоматические пути к циклу N были бы либо

for (int i = 0; i < N; i++)

или

for (int i = 1; i <= N; i++)

Таким образом, смешение двух идиом дает неправильное представление о том, сколько раз цикл будет выполнен. Для ясности я предлагаю

for (int i = 0; i < MAX_ATTEMPTS; i++) {
    game.roll(i/10);
}
// Game just ended.  Exception thrown here...
game.roll(2);

@Test(expected = BowlingGameScoreBoard.BowlingException.class)
public void testIllegalBowlException() {

        game.roll(200);

}

Это единственный безупречный тест. Даже тогда 200 кажется немного чрезмерным для незаконной ценности.

Заключение об модульных тестах

Обычно ошибки в модульных тестах могут не быть непосредственной причиной дисквалификации кандидата. Тем не менее, у вас почти каждый из них ошибочный, что, по-видимому, указывает на проблемы связи или непротиворечивость.

ответил 200_success 12 PM000000120000000831 2014, 12:28:08

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132