Pull to refresh

Code Review case 1

Reading time 4 min
Views 4.4K
Я работал в компании, в которой отсутствует практика code review. Для самосовершенствования и расширения кругозора я бы хотел получить немного конструктивной критики.

Сейчас предлагаю разобрать повторяющийся случай с обилием ветвлений.

Задание


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

Если пользователь перетащил и отпустил из папки в папку и другая папка находится на другом разделе, то проверить возможность копирования. Если скопировать можно, то скопировать. Иначе выдать сообщение, что нельзя скопировать. Невозможно скопировать бывает по причинам: нет прав на запись; не хватает свободного места; файловая система не поддерживает символы в имени; имя файла в папке назначения будет иметь слишком длинный путь; в папке уже есть файл с таким именем (вызвать диалог на перезапись файла, если пользователь согласен, то перезаписать).

Если папка назначения находится на том же разделе, что и файл, то переместить файл. Невозможно переместить: нет прав на запись; полный путь назначения окажется слишком длинным, в папке уже есть файл с тем же именем (вызвать диалог); файл системный и его нельзя удалить; уже есть файл с таким имененем (вызвать диалог на перезапись файла, если пользователь согласен, то перезаписать).

Если пользователь перенес файл в другой окно, но оно имеет тот же путь, то создать копию файла (Добавить к имени « копия #», где # – наименьшее положительное число, делающее файл уникальным). Невозможно создать копию: нет прав на запись; полный путь слишком длинный; не хватает свободного места.

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

Если пользователь отпустил файл в том же окне (файл сорвался) левой кнопкой, то ничего не делать. А если правой, то предложить создать копию или ярлык. Если файл сорвался не в окне папки, то ничего не делать.

Со временем могут появиться новые условия, новые действия, могут измениться уже описанные условия для выбора действий.

Решение для обсуждения


Предлагаю своё спорное решение на Java, в котором я достиг второго наибольшего уровня вложенности if:

public static void dispatchFileDropping(
        FileDragNDropEvent event
) {
    
    //------------------------------------------------------
    // Исходные предикаты 
    //  (имена даны в стиле математической логики).
    
    boolean A = isTargetPlaceIsDirectoryWindow(event);
    boolean B = isTargetDirEqualsSourceDir(event);
    boolean C = isTargetVolumeEqualsSourceVolume(event);
    boolean D = isMouseRightButtonUsed(event);
    boolean E = isSystemFileDroped(event);
    boolean F = isTargetVolumeHasFreeSpace(event);
    boolean G = isTargetDirWritable(event);
    boolean H = isSourceDirCleanable(event);
    boolean I = isFileNameOkForTarget(event);
    boolean J = isNewFileFullPathOkForTargetLimit(event);
    boolean K = isTargetDirHasSameNamedFile(event);
    boolean L = isTargetDirSameNamedFileIsWritable(event);
    
    Actions userChoise 
        = (A & D) ? askUserForAction(event) : null;
    if (userChoise == Actions.CANCEL) return;
    
    boolean M = (userChoise == Actions.COPY);
    boolean N = (userChoise == Actions.CLONE);
    boolean O = (userChoise == Actions.MOVE);
    boolean P = (userChoise == Actions.LINK);
    
    
    //------------------------------------------------------
    // С каким случаем имеем сейчас дело.
    
    boolean copyCase = (M & !K) | (A & !B & !C & !D & !K);
    boolean copyRewriteCase 
        = (M & K) | (A & !B & !C & !D & K);
    boolean cloneCase = N | (A & B & !D);
    boolean moveCase = (O & !K) | (A & !B & C & !D & !K);
    boolean moveRewriteCase = (O & K) | (A & !B & C & !D & K);
    boolean createLinkCase = P;
    
    
    //------------------------------------------------------
    // Пользователь может отказаться от перезаписи 
    //  существующего файла.
    
    if (copyRewriteCase | moveRewriteCase) {
        if (askUserWantToRewrite() == Answers.NO) return;
    }
    
    
    //------------------------------------------------------
    // Вычисляем возможность для каждого случая.
    
    boolean isPossibleToCopy = F & G & I & J;
    boolean isPossibleToCopyRewrite = isPossibleToCopy & L;
    boolean isPossibleToClone = isPossibleToCopy;
    boolean isPossibleToMove = isPossibleToCopy & !E & H;
    boolean isPossibleToMoveRewrite = isPossibleToMove & L;
    boolean isPossibleToLink = isPossibleToCopy & !K;
    
    
    //------------------------------------------------------
    // Либо выполняем операцию, либо объясняем, 
    //  почему не выполнили ее.
    
    String errorMessage = "";
    if (copyCase & !isPossibleToCopy) {
        errorMessage = "Невозможно скопировать файл.";
    } else if (copyRewriteCase & !isPossibleToCopyRewrite) {
        errorMessage = "Невозможно перезаписать файл.";
    } else if (cloneCase & !isPossibleToClone) {
        errorMessage = "Невозможно создать копию.";
    } else if (moveCase & !isPossibleToMove) {
        errorMessage = "Невозможно переместить файл.";
    } else if (moveRewriteCase & !isPossibleToMoveRewrite) {
        errorMessage 
            = "Невозможно переместить файл с заменой.";
    } else if (createLinkCase & !isPossibleToLink) {
        errorMessage = "Невозможно создать ссылку.";
    }
    
    String reasons = " Причины: \n";
    if (!F) { 
        reasons += "-- закончилось место на диске \n";
    }
    if (!G) {
        reasons 
        += "-- нет прав на запись в папке назначения \n";
    }
    if (!I) {
        reasons 
        += "-- имя файла содержит недопустимые символы \n";
    }
    if (!J) {
        reasons
        += "-- полный путь нового файла превышает предел \n";
    }
    if (moveCase | moveRewriteCase) {
        if (E) {
            reasons 
            += "-- нельзя удалить системный файл \n";
        }
        if (!H) {
            reasons 
            += "-- нет прав на удаление в папке \n";
        }
    } else if (copyRewriteCase | moveRewriteCase) {
        if (!L) {
            reasons 
            += "-- нет прав на изменение файла \n";
        }
    } else if (createLinkCase) {
        if (K) {
            reasons 
            += "-- файл с таким именем уже существует \n";
        }
    } 
    
    if (errorMessage.isEmpty()) {
        if (copyCase) copy(event);
        if (copyRewriteCase) copyRewrite(event);
        if (cloneCase) clone(event);
        if (moveCase) move(event);
        if (moveRewriteCase) moveRewrite(event);
        if (createLinkCase) createLink(event);
    } else {
        showMessage(errorMessage + reasons);
    }
}
Tags:
Hubs:
+3
Comments 30
Comments Comments 30

Articles