VPF::рефакторинг кода в стиле java8 - Форум программистов

Pawl

Дата 29.5.2014, 08:58 (ссылка)

(нет голосов)

Загрузка ...

Опытный

Профиль
Группа: Участник
Сообщений: 649
Регистрация: 22.4.2008
Где: Витебск

Репутация: 7
Всего: 28

Доброго времени суток. Попытался написать программку-архиватор, которая ищет в каталоге определенные файлы и пакует их в архив. Для архивации использую zip4j. Для лучшего усвоения кодирования в стиле java8 попытался использовать максимальное количество известных мне фич: функциональный интерфейс, стримы, лямбды и пр:

Код


import net.lingala.zip4j.core.ZipFile;
import net.lingala.zip4j.exception.ZipException;
import net.lingala.zip4j.model.ZipParameters;

import java.io.File;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.RecursiveAction;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toList;
import static net.lingala.zip4j.util.Zip4jConstants.COMP_STORE;

public class FilesArchiver extends RecursiveAction {
    private static final Lock MY_LOCK = new ReentrantReadWriteLock().writeLock();
    private static final ZipParameters PARAMETERS = new ZipParameters();
    private static final AtomicInteger COUNT = new AtomicInteger(0);
    private static ZipFile archive;
    private String path;

    public FilesArchiver(String dirPath, String archPath) {
        PARAMETERS.setCompressionMethod(COMP_STORE);
        path = dirPath;
        try {
            archive = new ZipFile(archPath);
        } catch (ZipException e) {
            System.out.println(e.getMessage());
        }
    }

    private FilesArchiver(String dirPath) {
        path = dirPath;
    }

    @Override
    public void compute() {
        File[] files = new File(path).listFiles();
        stream(files).parallel().filter(f -> f.isFile() && f.getName().endsWith("fb2"))
                .forEach(f -> withLock(() -> {
                    archive.addFile(f, PARAMETERS);
                    COUNT.getAndIncrement();
                }));
        invokeAll(stream(files).parallel().filter(f -> f.isDirectory())
                .map(f -> new FilesArchiver(f.getPath())).collect(toList()));
    }

    private static void withLock(Archiver action) {
        try {
            MY_LOCK.lock();
            action.doArchive();
        } catch (ZipException e) {
            System.out.println(e.getMessage());
        } finally {
            MY_LOCK.unlock();
        }
    }

    @FunctionalInterface
    private static interface Archiver {
        public void doArchive() throws ZipException;
    }

    public static void main(String[] args) {
        try {
            new ForkJoinPool().invoke(new FilesArchiver("d:\\Books\\Мастера фэнтези\\",
                    "D:\\tmp\\archive.zip"));
            System.out.println("archived files: " + COUNT.intValue());
        } catch (Exception e) {
            System.out.println("Wrong Directory");
        }
    }
}

Здесь мне не нравится то, что я создаю 2 стрима - для поиска вложенных каталогов и для архивирования. Из-за этого программка работает медленнее, чем такая же, но в стиле java7. Может, посоветуете, как тут можно обойтись 1 стримом, ну и вообще улучшить код? Спасибо!

--------------------

В действительности всё совсем не так, как на самом деле

Kircul

Дата 3.6.2014, 01:13 (ссылка)

(нет голосов)

Загрузка ...

Бывалый

Профиль
Группа: Участник
Сообщений: 166
Регистрация: 20.2.2007

Репутация: 6
Всего: 7

1. Не понял зачем распаралеливать поток (stream) файлов fb2 если потом все равно вся обработка (самая затратная операция) проходит в один поток (thread).
2. Похожая проблема со вторым потоком (stream) (директорий). Может делать в один поток (thread) и не вызывая collect запускать FilesArchiver.compute прямо в потоке (stream)?

В общем не вижу смысла играться с многопоточностью (thread) когда алгоритм по сути однопоточен (если я конечно правильно его понял).
А можно увидеть код в java 7 стиле? Для сравнения...

Updated: добавил пояснение к словам "поток", где я подразумеваю stream а где thread.

Это сообщение отредактировал(а) Kircul - 3.6.2014, 01:28

Kircul

Дата 3.6.2014, 02:53 (ссылка)

(нет голосов)

Загрузка ...

Бывалый

Профиль
Группа: Участник
Сообщений: 166
Регистрация: 20.2.2007

Репутация: 6
Всего: 7

И вот еще забыл, фильтровать файлы лучше при вызове метода listFiles

У меня нет под рукой компилятора java 8, по этому не гарантирую работу, но выглядеть должно как-то так:

Код


        // К черту withLock и parallel
        stream(new File(path).listFiles(f -> f.isFile() && f.getName().endsWith("fb2")))
                .forEach(f -> {
                    archive.addFile(f, PARAMETERS);
                    COUNT.getAndIncrement();
                });
        // т.к. суть всего рефакторинга по максимуму использовать новые возможности Java 8, можно попробовать поиграть с сылками на методы и композицией функций
        stream(new File(path).listFiles(File::isDirectory))
                .map(((Function<File, String>) File::getPath).andThen(FilesArchiver::new))
                .forEach(FilesArchiver::collect);

Это сообщение отредактировал(а) Kircul - 3.6.2014, 04:00

Pawl

Дата 3.6.2014, 08:22 (ссылка)

(нет голосов)

Загрузка ...

Опытный

Профиль
Группа: Участник
Сообщений: 649
Регистрация: 22.4.2008
Где: Витебск

Репутация: 7
Всего: 28

Цитата(Kircul @ 3.6.2014, 01:13

)

А можно увидеть код в java 7 стиле? Для сравнения

Вот он:

Код


import net.lingala.zip4j.core.ZipFile;
import net.lingala.zip4j.exception.ZipException;
import net.lingala.zip4j.model.ZipParameters;
import net.lingala.zip4j.util.Zip4jConstants;

import java.io.*;
import java.util.ArrayList;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.RecursiveAction;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class FilesArchiver extends RecursiveAction {
    private static final Lock w = new ReentrantReadWriteLock().writeLock();
    private static final AtomicInteger COUNT = new AtomicInteger(0);
    private static final ZipParameters PARAMETERS = new ZipParameters();
    private static ZipFile archive;
    private String path;

    static {
        try {
            archive = new ZipFile("D:\\temp\\archive.zip");
            PARAMETERS.setCompressionMethod(Zip4jConstants.COMP_STORE);
        } catch (ZipException e) {
        }
    }

    public FilesArchiver(String path) {
        this.path = path;
    }

    @Override
    public void compute() {
        ArrayList<FilesArchiver> tasks = new ArrayList<>();
        for (File file : new File(path).listFiles()) {
            if (file.isDirectory()) {
                tasks.add(new FilesArchiver(file.getPath()));
            } else {
                if (file.getName().endsWith("fb2")) {
                    try {
                        w.lock();
                        archive.addFile(file, PARAMETERS);
                        w.unlock();
                    } catch (Exception e) {
                        System.err.println(e.getMessage());
                    }
                    COUNT.getAndIncrement();
                }
            }
        }
        invokeAll(tasks);
    }

    public static void main(String[] args) {
        try {
            new ForkJoinPool().invoke(new FilesArchiver("d:\\Books\\Мастера фэнтези\\"));
            System.out.println("archived files: " + COUNT.intValue());
        } catch (Exception e) {
            System.out.println("Wrong Directory");
        }
    }
}

Цитата(Kircul @ 3.6.2014, 01:13

)

когда алгоритм по сути однопоточен

я использую fork / join, так что происходит параллельный рекурсивный поиск файлов по всему каталогу и подкатологам и их упаковка несколькими thread'ами. Поэтому, кстати, и использую локер - чтобы, пока один thread пишет в архив, остальные его в это время не трогали, т. к. класс ZipFile is not Thread safety.

Цитата(Kircul @ 3.6.2014, 02:53

)

можно попробовать поиграть с сылками на методы

Красиво! Только не ясны 2 момента:
1)

Код


.andThen(FilesArchiver::new))

в конструктор FilesArchiver'a у меня передаётся аргумент - строка с путём к к аталогу, в котором надо искать файлы для архивирования. Она что же, автоматически туда попадёт после

Код


.map(((Function<File, String>) File::getPath)

?
2)

Код


.forEach(FilesArchiver::collect)

эта строка у меня не компилится: пишет, что не может найти метод collect. Я так понимаю, что ссылки на методы работают, если эти методы в классе есть, но в FilesArchiver'e нет метода collect. Его нужно создать?

--------------------

В действительности всё совсем не так, как на самом деле

Kircul

Дата 3.6.2014, 10:15 (ссылка)

(нет голосов)

Загрузка ...

Бывалый

Профиль
Группа: Участник
Сообщений: 166
Регистрация: 20.2.2007

Репутация: 6
Всего: 7

Цитата

я использую fork / join, так что происходит параллельный рекурсивный поиск файлов по всему каталогу и подкатологам и их упаковка несколькими thread'ами.

Нет. "listFiles" однопоточен, само архивирование тоже. А то что фильтрация элементов в списке проводится в несколько потоков может вовсе и не ускорять выполение алгоритма.
Советую попробовать для сравнения однопоточный вариант (который я описал выше) и по профайлить.

Цитата

эта строка у меня не компилится: пишет, что не может найти метод collect.

Описка, я имел в виду метод compute.

Цитата

Она что же, автоматически туда попадёт после

http://docs.oracle.com/javase/8/docs/api/j...ction.Function-
По сути:

Код


a.andThen(b)

эквивалентно:

Код


b(a(...))

Пруф: http://hg.openjdk.java.net/lambda/lambda/j...nction.java#l86

Pawl

Дата 3.6.2014, 17:41 (ссылка)

(нет голосов)

Загрузка ...

Опытный

Профиль
Группа: Участник
Сообщений: 649
Регистрация: 22.4.2008
Где: Витебск

Репутация: 7
Всего: 28

Цитата(Kircul @ 3.6.2014, 10:15

)

А то что фильтрация элементов в списке проводится в несколько потоков может вовсе и не ускорять выполение алгоритма.

Действительно,

Цитата(Kircul @ 3.6.2014, 01:13

)

код в java 7 стиле

работает не быстрее, чем ваш вариант. Интересно было бы попробовать заархивировать тысяч 5 файлов - для сравнения... В любом случае, спасибо за ответы и разъяснения! smile

--------------------

В действительности всё совсем не так, как на самом деле

0 Пользователей читают эту тему (0 Гостей и 0 Скрытых Пользователей)
0 Пользователей:
« Предыдущая тема \| Java: Общие вопросы \| Следующая тема »