Имеет ли эта конструкция смысл? - PullRequest
0 голосов
/ 22 января 2020

Столкнулся с проектом с этим кодом:

   public class IndexUpdater implements Runnable {
        @Override
        public void run() {
            final AtomicInteger count = new AtomicInteger(0);
            FindIterable<Document> iterable = mongoService.getDocuments(entryMeta, null, "guid");
                iterable.forEach(new Block<Document>() {
                    @Override
                    public void apply(final Document document) {
                        count.incrementAndGet();
                        // A lot of code....
                        if (count.get() / 100 * 100 == count.get()) {
                            LOG.info(String.format("Processing: %s", count.get()));
                        }
                    }
                });
        }
   }

Здесь меня интересуют три строки кода:

if (count.get() / 100 * 100 == count.get()) {
    LOG.info(String.format("Processing: %s", count.get()));
}

Имеет ли это условие смысл с учетом многопоточности и типа Переменная AtomicInteger? Или это бессмысленная проверка? Интересно, что IntellijIdea не подчеркивает эту конструкцию как бессмысленную.

1 Ответ

5 голосов
/ 22 января 2020

Я бы не назвал этот код "бессмысленным", а скорее неправильно (или, возможно, он имеет непреднамеренную семантику).

Если бы он вызывался многопоточным способом, вы не всегда получите одно и то же значение для count.get() на трех вызовах метода (их будет четыре, если вы включите count.incrementAndGet()).

Последствия этого не выглядят катастрофическими c в этом случае - вы, возможно, пропустите несколько операторов регистрации, и вы можете увидеть некоторые неожиданные сообщения, такие как Processing 101, а затем удивиться, почему число не кратно 100. Но, возможно, если вы Если бы мы использовали такую ​​же конструкцию в другом месте, это привело бы к более значительным последствиям.

Поместите результат count.incrementAndGet() в переменную (*), чтобы потом можно было его использовать.

Но тогда это было бы проще также использовать count.get() % 100 == 0:

int value = count.incrementAndGet();
// A lot of code....
if (value % 100 == 0) {
  LOG.info(String.format("Processing: %s", value));
}

, что является одновременно правильным (или, вероятно, тем, что предназначено) и более легким для чтения.


(* ) В зависимости от того, что вы действительно хотите показать с этим журналом В этом сообщении вы можете поставить count.incrementAndGet() после «Много кода».

...