Code Review case 1

    I worked for a company that lacks the practice of code review. For self-improvement and broadening of horizons, I would like to get some constructive criticism.

    Now I propose to disassemble the recurring case with an abundance of branches.

    The task

    The user intends to drag the file with the mouse from one folder window to another. You need to write a dispatch method that checks the essence of the event and the possibility of its processing, if necessary, clarifies the details, then calls the required method or displays a message about restrictions.

    If the user dragged and released from a folder to a folder and another folder is on a different section, then check the possibility of copying. If you can copy, then copy. Otherwise, issue a message that can not be copied. Impossible to copy happens for reasons: no rights to write; not enough free space; the file system does not support characters in the name; the file name in the destination folder will be too long; there is already a file with the same name in the folder (call the dialog to overwrite the file, if the user agrees, overwrite).

    If the destination folder is on the same section as the file, then move the file. Unable to move: no write permission; the full destination path will be too long, there is already a file in the folder with the same name (call the dialog); the file is system and cannot be deleted; there is already a file with such a name (call a dialog to overwrite the file, if the user agrees, overwrite).

    If the user has moved the file to another window, but it has the same path, then create a copy of the file (Add to the name “copy #”, where # is the smallest positive number that makes the file unique). Unable to create copy: no write permission; full path is too long; not enough free space.

    If the user transferred the right button, then call the dialog to select the action (copy / move / create a shortcut / create a copy).

    If the user released the file in the same window (the file broke) with the left button, then do nothing. And if the right, then offer to create a copy or shortcut. If the file is not broken in the folder window, then do nothing.

    Over time, new conditions may appear, new actions, conditions for the selection of actions that have already been described may change.

    Decision to discuss

    I offer my controversial Java solution, in which I reached the second highest if nesting level:

            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 = "Невозможно скопировать файл.";
        } elseif (copyRewriteCase & !isPossibleToCopyRewrite) {
            errorMessage = "Невозможно перезаписать файл.";
        } elseif (cloneCase & !isPossibleToClone) {
            errorMessage = "Невозможно создать копию.";
        } elseif (moveCase & !isPossibleToMove) {
            errorMessage = "Невозможно переместить файл.";
        } elseif (moveRewriteCase & !isPossibleToMoveRewrite) {
                = "Невозможно переместить файл с заменой.";
        } elseif (createLinkCase & !isPossibleToLink) {
            errorMessage = "Невозможно создать ссылку.";
        String reasons = " Причины: \n";
        if (!F) { 
            reasons += "-- закончилось место на диске \n";
        if (!G) {
            += "-- нет прав на запись в папке назначения \n";
        if (!I) {
            += "-- имя файла содержит недопустимые символы \n";
        if (!J) {
            += "-- полный путь нового файла превышает предел \n";
        if (moveCase | moveRewriteCase) {
            if (E) {
                += "-- нельзя удалить системный файл \n";
            if (!H) {
                += "-- нет прав на удаление в папке \n";
        } elseif (copyRewriteCase | moveRewriteCase) {
            if (!L) {
                += "-- нет прав на изменение файла \n";
        } elseif (createLinkCase) {
            if (K) {
                += "-- файл с таким именем уже существует \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);

    Also popular now: