java-park-mail-ru / 2016-02-unnamed-1-bmp

Азартная игра
0 stars 1 forks source link

РК3 #10

Closed ValerGit closed 8 years ago

ValerGit commented 8 years ago

Готово

ValerGit commented 8 years ago

@esin88 сегодня уже рк 😢 пять дней никакой обратной связи(

oxeg commented 8 years ago

Прошу прощения, завалился делами на работе. Не переживайте, все проверю

oxeg commented 8 years ago

frontend.GameWebSocketCreator.AnimalPlayer#VALUES - можно выпилить, лишнее. получение рандомного имени можно заменить на

        private static final int SIZE = AnimalPlayer.values().length;
        private static final Random RANDOM = new Random();

        public static AnimalPlayer randomAnimal() {
            return AnimalPlayer.values()[RANDOM.nextInt(SIZE)];
        }
oxeg commented 8 years ago

dbService/DBServiceImpl.java:56 - такие вещи нужно заменить на try with resources

oxeg commented 8 years ago

Ну и инспекции нужно бы починить https://s.mail.ru/CGvG/mWd8TKG9h

ValerGit commented 8 years ago

dbService/DBServiceImpl.java:56

Константин считает иначе...кому верить? см. один из комментариев к пулл реквесту 2го рк:

Как написано в документации http://docs.jboss.org/hibernate/orm/5.1/userguide/html_single/Hibernate_User_Guide.html#session-per-operation каждый участок кода, работающий с базой данных (в оригинале Unit of work), должен делать это в транзакции. Кроме того вы тут не закрываете сессию в случае исключения. Я думаю вы уже поняли, что если в каждом методе создавать сессию, открывать транзакцию, комитить её, и т.д., то все методы будут похожи как братья-близнецы. Это хороший признак того, что вам нужно разделить класс на несколько. В одном (пусть это будет dbService) оставим только обвязку по работе с базой. Во втором (UserService) останутся функции оперирования пользователем. Выглядеть это должно примерно так:

@FunctionalInterface public interface HibernateUnit { @Nullable T operate(@NotNull Session session); } public final class dbService { ...

@Nullable public T doReturningWork(@NotNull HibernateUnit work) throws DatabaseException{ final Session session = sessionFactory.openSession(); try { session.getTransaction().begin(); T result = work.operate(session); session.getTransaction().commit(); return result; } catch ( RuntimeException e ) { if ( session.getTransaction().getStatus() == TransactionStatus.ACTIVE || session.getTransaction().getStatus() == TransactionStatus.MARKED_ROLLBACK ) { session.getTransaction().rollback(); } throw new DatabaseException(e); } finally { session.close(); } } после чего функция saveUser преобразится в:

@Override
public long saveUser(UserDataSet dataSet) throws DatabaseException {
    //noinspection ConstantConditions
    return dbService.doReturningWork((session)-> {
        final UserDataSetDAO dao = new UserDataSetDAO(session);
        final Long returnedId = dao.save(dataSet);
        if (returnedId == -1L) {
            LOGGER.error("Fail to add new user");
            return -1L;
        }
        return returnedId;
    });
}

Вам нужно написать еще одну функцию в dbService void doWork(Consumer) throws DatabaseException, которая делает что-то в базе не возвращая ничего. З.ы. DatabaseException - тоже нужно написать.

oxeg commented 8 years ago

Не вижу здесь противоречия с тем, что говорил Константин. У вас конструкция

        final Session session = sessionFactory.openSession();
        try {
...
        } finally {
            session.close();
        }

отлично заменятеся на try with resources. Более того, в этом месте горит инспекция что в finally блоке возможно исключение.

ValerGit commented 8 years ago

Я прочитала документацию гибернейта и у меня возник вопрос, если каждая сессия будет autoclosable и метод doReturningWork переписать следующим образом:

    @Override
    @Nullable
    public <T>T doReturningWork(@NotNull HibernateUnit<T> work) throws DatabaseException {
        try( Session session = sessionFactory.openSession()) {
            session.getTransaction().begin();
            final T result = work.operate(session);
            session.getTransaction().commit();
            return result;
        } catch (HibernateException e) {
            throw new DatabaseException("Fail to perform a transaction",e);
        }
    }

Будет ли текущая транзакция откатываться автоматически?... так как конструкция прошлой версии метода (см. ниже) здесь недоступна в силу использования autoclosable


catch ( RuntimeException e ) {
            if ( session.getTransaction().getStatus() == TransactionStatus.ACTIVE
                    || session.getTransaction().getStatus() == TransactionStatus.MARKED_ROLLBACK ) {
                session.getTransaction().rollback();
            }
ValerGit commented 8 years ago

Изменила генерацию имен для пользователей и починила инспекции. С autoclosable исправила, но не уверена на счет правильности ☺️

ValerGit commented 8 years ago

Думаю, что раз в 4 дня прилично напоминать о себе :bowtie:

oxeg commented 8 years ago

Не знаю.. Надо уточнить у вышестоящей инстанции :neckbeard: А вообще, лучше бы на лекции ходили. Вчера была очень важная лекция, про многопоточность. Ваш пул реквест у меня висит в "на посмотреть". Постараюсь сегодня, в крайнем случае завтра точно проверю.

ValerGit commented 8 years ago

Это была первая лекция, которую я пропустила - ангина :mask: Буду ждать проверки)

oxeg commented 8 years ago

Проверю-проверю. Не переживайте и выздоравливайте :godmode:

oxeg commented 8 years ago

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

        Transaction transaction = null;
        try (Session session = sessionFactory.openSession()) {
            transaction = session.getTransaction();
            ...
        } catch (HibernateException e) {
            if (transaction != null) transaction.rollback();
            ...
        }