Code Review Java?

Question

[Toseter]

Дано: имеется open source проект, в зачаточном состоянии. И прежде чем продолжать хотелось бы понять не накосячил ли где то в плане кода и архитектуры, ибо есть места в которых я очень сильно сомневаюсь.


Собственно вопрос: к кому обратится человек/сервис?


P.S. линк на проект github.com/Toseter/SeleniumRunner/tree/develop

в readme и wiki написано, что собственно это за проект, и что уже реализовано.

Answer 1

[serso]

Пожалуй начну жёсткую критику:
1. Проект под мавеном — это хорошо, но плохо что он не поддерживает
стандартную структуру каталогов проекта
2. Config.parseConfiguration() — зачем самомоу парсить? Для этого есть уже написанные библиотеки типа Commons CLI для работы с коммандной строкой и Commons Configuration для хранения настроек (естественно, это можно заменить на что-нибудь другое но посыл, я думаю, понятен)
3. Java code conventions нужно поддерживать — название класса isCloneable, имя пакета parserHandlers и

private List<SuiteResult> _results;

не есть хорошо
4.

if ( ... && command != -2)

— это ад. Что значит константа -2?
5. Нет такого слова Formater, нужно Formatter
6.

stringBuilder.append("[" + result.getTestSuiteName() + "]\r\n")

— используете StringBuilder и зачем-то конкатенируете строки
7.

if (Config.isRunnable() == false)

— так не пишут, используйте операцию отрицания.
8. Вместо ParserHelper и статичных методов опишите интерфейс — мало ли откуда входные данные придётся парсить (Естественно, в качестве дефолтной реализации используйте свой код для парсинга из xml)
9.

 public Command[] getCommands() {
        return commands.toArray(new Command[commands.size()]);
    }

— почему бы список не возвращать? Вам действительно где-то нужен массив?
10.

                    } catch (CloneNotSupportedException ex) {

— выбрасывние CloneNotSupportedException — ошибра разработчиков java, с своих классах рекомендуется убирать из сигнатуры.

Я думаю остановлюсь на этом. В плане архитектуры настоятельно рекомендую глянуть как реализована, например, библиотека jUnit

Comments

[serso]

Java-way

Опишите интерфейс:

interface Command {
    Result run(WebDriver webDriver, State state, Map<String, String> params) throw IllegalArgumentException;
}

Сервис:

interface CommandsService {
    @Nullable
    Command getCommand(@NotNull String commandName);

    Result runCommand(@NotNull String commandName, // PARAMS);

    void registerCommand(@NotNull String commandName, @NotNull Command command);
}

Это даст возможность расширять базовый фукнкционал библиотеки (т.е. программисты-пользователи смогут свои реализации Command писать и использовать).

Реализация built-in комманд делается через enum следующим способом (который, кстати, описан Джошуа Блохом):

public enum DefaultCommand implements Command{      

    open{
        Result run(WebDriver webDriver, State state, Map<String, String> params) throw IllegalArgumentException {
            // do logic
        }
    },

    type {
        Result run(WebDriver webDriver, State state, Map<String, String> params) throw IllegalArgumentException {
            // do logic
        }
    };
}

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

Надеюсь, мысль ясна

Answer 2

[Disturbed]

от авторов StackOverflow :)
codereview.stackexchange.com/

Comments

[Toseter]

спасибо, там в основном конкретные куски кода выкладываются, а мне бы проект в целом.

Answer 3

[evilbloodydemon]

может быть спросить у разработчиков селениума?