Вы все пишите блистательно,
а я потом добавлю шероховатости.
х/ф Трамбо
Написать Java код не просто, а очень просто. Трудности начинаются, когда его запускают или, хуже того, если его требуется изменить. Открыв свой код двухлетней давности, каждый хотя бы раз задавался вопросом: кто же все это написал? Мы в Wrike разрабатываем продукт и делаем это уже более десяти лет. Подобные ситуации случались с нами неоднократно. За это время мы выработали ряд принципов в написании кода, которые помогают нам сделать его проще и нагляднее. Хотя в нашем подходе нет ничего экстраординарного, он во многом отличается от того, как принято писать код на Java. Тем не менее, кто-то может найти нечто полезное в нашем подходе и для себя.
Практика показывает, что код становится существенно проще, если для каждого метода как его параметры, так и результат являются неизменяемыми.
Более того, практически всегда можно добиться того же для всех локальные значений методов или полей классов. Логично, при этом, что у класса могут быть только getters. Как быть, если нужно сконструировать сложную модель данных: создавать билдер имеющий только setters и внутри него уже конструировать неизменяемый объект методом build().
Такой подход действительно приводит к более простому и читаемому коду, но чтобы увидеть это, нужно попробовать. Вот несколько примеров в поддержку работы с неизменяемым состоянием. Язык Rust по умолчанию определяет все “переменные” неизменяемыми:
а для того чтобы получить именно переменную, нужно приложить дополнительные усилия:
В языке Erlang вообще нет переменных, абсолютно все значения являются неизменяемыми, тем не менее на нем можно разрабатывать сложные приложения. У Robert C. Martin есть интересное выступление на эту тему с хорошо изложенной теорией и не столь хорошими примерами, но теория все равно стоит того, чтобы посмотреть.
Открыв файл, его нужно закрыть. Получив соединение с базой, его нужно освободить. В целом ничего хитрого. Но практика показывает, что при работе с ресурсами Java-разработчики склонны допускать ошибки, вероятно, потому что мы лишены радости управлять памятью вручную. Навык не вырабатывается. Простой пример:
В коде выше есть неочевидная ошибка, из за которой FileInputStream может быть не закрыт.
Простое и дешевое решение — лишить программиста необходимости управлять ресурсами вообще. Например так:
В этом случае, даже если где-то в логике обработки файла закрадется ошибка, по крайней мере все ресурсы будут корректно освобождены и закрыты.
Особенно это заметно на интервью, когда начинающие программисты впадают в ступор, если от них требуется вернуть из метода сразу два значения, скажем, сумму и среднее. Решения варьируются по степени своего неприличия от
или
до
При этом аккуратное решение совсем простое:
Совершенно нормально, и более того, удобно, когда вспомогательные модели данных определены рядом с методами, которые их используют. Так принято поступать абсолютно во всех языках программирования, даже в C#, который по большому счету является клоном Java. Однако среди Java программистов можно столкнуться со странным заблуждением, что, мол, “каждый класс должен быть определен в отдельном файле, иначе это плохой код, и точка”. В Java действительно было нельзя создавать inner классы, в версии до 1.1, но с 19 Февраля 1997 года эта проблема была решена, и ничто не мешает нам пользоваться этой новой возможностью языка, которая доступна уже как двадцать лет.
Бывает, что в процессе реализации метода возникает необходимость разбить его на несколько более простых методов. При этом неизбежно возникает вопрос: где эти вспомогательные методы определить? Конечно, можно нарезать логику на приватные методы, но если класс достаточно большой, это становится неудобно. Приватные методы неизбежно получаются разбросанными по всему файлу, и какой из них для чего нужен и где используется становится не так очевидно, как хотелось бы. Простое решение, которое помогает как аккуратно организовать код, так и меньше скролить файл при его чтении, — определить вспомогательные методы внутри основного публичного метода.
Для примера, ниже приведен код обхода графа в глубину, при этом для каждого узла графа вызывается callback функция, естественно ровно один раз для каждого узла.
Можно было бы определить рядом вспомогательный метод:
Но такое решение получается достаточно громоздким, а сам вспомогательный метод — одиноким. Например, глядя только на название метода stepToNodeImpl неочевидно, что он на самом деле используется для обхода графа в глубину. Чтобы разобраться в этом, нужно сначала найти все его использования.
Тут может быть разумная критика, что много логики будет спрятано внутри метода и отдельно ее невозможно будет тестировать, справедливо. Вопрос, действительно ли все детали реализации, включая приватные методы, покрываются отдельными тестами? Если да, вероятно, логику придется декомпозировать в угоду тестам. Если нет, уж лучше организовать код более аккуратным способом.
На практике, в зависимости от ситуации, бывает удобно реализовывать внутреннюю логику метода используя Runnable, Callable, Consumer или Supplier интерфейсы.
Вычисление отдельного значение может оказаться нетривиальной задачей, если оно вычисляется по сложным правилам или требует дополнительных промежуточных вычислений. В результате код может получиться “замусоренным”. Скажем, дальнейшая логика метода зависит от того, поддерживается ли версия браузера клиента или нет. Для этого нам вначале нужно будет определить тип браузера, затем, в зависимости от типа сравнить версии, и, возможно, учесть какие-то дополнительные параметры.
Примечание: конечно, в данном примере подразумевается, что логика вычисления поддерживаемого клиента сугубо специфична, в противном случае ее само собой нужно вынести в отдельный вспомогательный метод.
Можно было бы вычислить значение isSupportedBrowser непосредственно в коде, но тогда в области видимости остались бы все вспомогательные переменные, а момент, где начинаются вычисления и где они завершаются, был бы не так очевиден. В примере выше конструкция:
Код не всегда бывает сложным, иногда его просто много. Задача переложить одну модель данных в другую скорее типична. Ну и что, что в итоге метод занимает пятьсот строк. Все же понятно. Берем это, кладем сюда, берем то, кладем туда и так далее. Встречаются редкие if или дополнительные вычисления, но погоды они не делают. При этом, если попытаться разбить код на более мелкие методы, может получиться даже хуже. Появляется необходимость передавать между между методами данные, определять дополнительные модели для промежуточных результатов методов и так далее.
Для организации такого кода могут быть крайне полезны пустые строки. Разбив код на параграфы, его можно сделать гораздо более читаемым. Но можно пойти дальше, и выделять осмысленные моменты в {} — фигурные скобки. Это не только поможет структурировать код блоками, но и спрятать из области видимости значения необходимые только для одной из секций.
Конечно, это не всегда лучший подход. Бывает что сколько {} скобок ни добавляй, код лучше не становится, и его просто нужно переписывать. Бывает, что все делится на методы вполне просто и замечательно. Тем не менее, использование {} позволяет добавить выразительности коду там, где это необходимо.
Любое приложение, каким бы большим или маленьким оно ни было, в конечном счете состоит из простых вещей: условий, циклов, выражений, моделей данных, в конечном счете строк кода. Как бы красиво ни называлась архитектура приложения, какие бы паттерны программирования и фреймворки в нем ни использовались, главное — то, насколько просто или сложно понять, как работает код, который перед тобой на экране. Приведенные выше примеры позволяют сделать код чуть менее сложным и чуть более выразительным. По крайней мере, для нас они полезны.
а я потом добавлю шероховатости.
х/ф Трамбо
Написать Java код не просто, а очень просто. Трудности начинаются, когда его запускают или, хуже того, если его требуется изменить. Открыв свой код двухлетней давности, каждый хотя бы раз задавался вопросом: кто же все это написал? Мы в Wrike разрабатываем продукт и делаем это уже более десяти лет. Подобные ситуации случались с нами неоднократно. За это время мы выработали ряд принципов в написании кода, которые помогают нам сделать его проще и нагляднее. Хотя в нашем подходе нет ничего экстраординарного, он во многом отличается от того, как принято писать код на Java. Тем не менее, кто-то может найти нечто полезное в нашем подходе и для себя.
Как использовать неизменяемые модели данных
Практика показывает, что код становится существенно проще, если для каждого метода как его параметры, так и результат являются неизменяемыми.
ImmutableList<Double> multiplyValuesByFactor(
final ImmutableList<Double> valueList,
final double factor) { … }
Более того, практически всегда можно добиться того же для всех локальные значений методов или полей классов. Логично, при этом, что у класса могут быть только getters. Как быть, если нужно сконструировать сложную модель данных: создавать билдер имеющий только setters и внутри него уже конструировать неизменяемый объект методом build().
Такой подход действительно приводит к более простому и читаемому коду, но чтобы увидеть это, нужно попробовать. Вот несколько примеров в поддержку работы с неизменяемым состоянием. Язык Rust по умолчанию определяет все “переменные” неизменяемыми:
let x = 5;а для того чтобы получить именно переменную, нужно приложить дополнительные усилия:
let mut y = 6; y=7;В языке Erlang вообще нет переменных, абсолютно все значения являются неизменяемыми, тем не менее на нем можно разрабатывать сложные приложения. У Robert C. Martin есть интересное выступление на эту тему с хорошо изложенной теорией и не столь хорошими примерами, но теория все равно стоит того, чтобы посмотреть.
It’s a Trap! Как избежать ручного управления ресурсами
Открыв файл, его нужно закрыть. Получив соединение с базой, его нужно освободить. В целом ничего хитрого. Но практика показывает, что при работе с ресурсами Java-разработчики склонны допускать ошибки, вероятно, потому что мы лишены радости управлять памятью вручную. Навык не вырабатывается. Простой пример:
try(final ZipInputStream zipInputStream =
new ZipInputStream(
new FileInputStream(file), charset)) {...}
В коде выше есть неочевидная ошибка, из за которой FileInputStream может быть не закрыт.
Простое и дешевое решение — лишить программиста необходимости управлять ресурсами вообще. Например так:
void processFile(final File file, FileProcessor fileProcessor) throws IOException {
try (final FileInputStream fileInputStream = new FileInputStream(file)) {
fileProcessor.processFile(fileInputstream);
}
}
В этом случае, даже если где-то в логике обработки файла закрадется ошибка, по крайней мере все ресурсы будут корректно освобождены и закрыты.
Как вернуть два значения
Особенно это заметно на интервью, когда начинающие программисты впадают в ступор, если от них требуется вернуть из метода сразу два значения, скажем, сумму и среднее. Решения варьируются по степени своего неприличия от
return new Pair<>(sum, avg)или
return new AbstractMap.SimpleEntry<>(sum, avg)до
return new Object[]{sum, avg}При этом аккуратное решение совсем простое:
public class MathExt {
public static class SumAndAverage {
private final long sum;
private final double average;
public SumAndAverage(final long sum, final double average) {...}
public long getSum() {...}
public double getAverage() {...}
}
public static SumAndAverage computeSumAndAverage(final ImmutableList<Integer> valueList) {
...
}
}
Совершенно нормально, и более того, удобно, когда вспомогательные модели данных определены рядом с методами, которые их используют. Так принято поступать абсолютно во всех языках программирования, даже в C#, который по большому счету является клоном Java. Однако среди Java программистов можно столкнуться со странным заблуждением, что, мол, “каждый класс должен быть определен в отдельном файле, иначе это плохой код, и точка”. В Java действительно было нельзя создавать inner классы, в версии до 1.1, но с 19 Февраля 1997 года эта проблема была решена, и ничто не мешает нам пользоваться этой новой возможностью языка, которая доступна уже как двадцать лет.
Как спрятать реализацию
Бывает, что в процессе реализации метода возникает необходимость разбить его на несколько более простых методов. При этом неизбежно возникает вопрос: где эти вспомогательные методы определить? Конечно, можно нарезать логику на приватные методы, но если класс достаточно большой, это становится неудобно. Приватные методы неизбежно получаются разбросанными по всему файлу, и какой из них для чего нужен и где используется становится не так очевидно, как хотелось бы. Простое решение, которое помогает как аккуратно организовать код, так и меньше скролить файл при его чтении, — определить вспомогательные методы внутри основного публичного метода.
Для примера, ниже приведен код обхода графа в глубину, при этом для каждого узла графа вызывается callback функция, естественно ровно один раз для каждого узла.
public static void traverseGraphInDepth(
final Graph graph,
final Consumer<Node> callback
) {
new Runnable() {
private final HashSet<Node> passedNodeSet = new HashSet<>();
public void run() {
for(final Node startNode:graph.listStartNodes()){
stepToNode(startNode);
}
}
void stepToNode(final Node node) {
if (passedNodeSet.contains(node)) {
return;
} else {
passedNodeSet.add(node);
callback.accept(node);
for(final Node nextNode:graph.listNextNodes(node)){
stepToNode(nextNode);
}
}
}
}.run();
}
Можно было бы определить рядом вспомогательный метод:
private static void stepToNodeImpl(
final Graph graph,
final Node node,
final Consumer<Node> callback,
final HashSet<Node> passedNodeSetMutable) {...}
Но такое решение получается достаточно громоздким, а сам вспомогательный метод — одиноким. Например, глядя только на название метода stepToNodeImpl неочевидно, что он на самом деле используется для обхода графа в глубину. Чтобы разобраться в этом, нужно сначала найти все его использования.
Тут может быть разумная критика, что много логики будет спрятано внутри метода и отдельно ее невозможно будет тестировать, справедливо. Вопрос, действительно ли все детали реализации, включая приватные методы, покрываются отдельными тестами? Если да, вероятно, логику придется декомпозировать в угоду тестам. Если нет, уж лучше организовать код более аккуратным способом.
На практике, в зависимости от ситуации, бывает удобно реализовывать внутреннюю логику метода используя Runnable, Callable, Consumer или Supplier интерфейсы.
Как вычислить значение
Вычисление отдельного значение может оказаться нетривиальной задачей, если оно вычисляется по сложным правилам или требует дополнительных промежуточных вычислений. В результате код может получиться “замусоренным”. Скажем, дальнейшая логика метода зависит от того, поддерживается ли версия браузера клиента или нет. Для этого нам вначале нужно будет определить тип браузера, затем, в зависимости от типа сравнить версии, и, возможно, учесть какие-то дополнительные параметры.
final boolean isSupportedBrowser = ((Supplier<Boolean>)()->{
final BrowserType browserType = …
final boolean isMobileBrowser = …
final boolean isTabletBrowser = …
final …
final …
if ( … && … ) {
return true;
}
if ( … || … ) {
return true;
} else {
if ( … ) {
return true;
…
return false;
}).get();
Примечание: конечно, в данном примере подразумевается, что логика вычисления поддерживаемого клиента сугубо специфична, в противном случае ее само собой нужно вынести в отдельный вспомогательный метод.
Можно было бы вычислить значение isSupportedBrowser непосредственно в коде, но тогда в области видимости остались бы все вспомогательные переменные, а момент, где начинаются вычисления и где они завершаются, был бы не так очевиден. В примере выше конструкция:
isSupportedBrowser = ((Supplier<boolean>)()->{ … }).get();Как структурировать скучный код
Код не всегда бывает сложным, иногда его просто много. Задача переложить одну модель данных в другую скорее типична. Ну и что, что в итоге метод занимает пятьсот строк. Все же понятно. Берем это, кладем сюда, берем то, кладем туда и так далее. Встречаются редкие if или дополнительные вычисления, но погоды они не делают. При этом, если попытаться разбить код на более мелкие методы, может получиться даже хуже. Появляется необходимость передавать между между методами данные, определять дополнительные модели для промежуточных результатов методов и так далее.
Для организации такого кода могут быть крайне полезны пустые строки. Разбив код на параграфы, его можно сделать гораздо более читаемым. Но можно пойти дальше, и выделять осмысленные моменты в {} — фигурные скобки. Это не только поможет структурировать код блоками, но и спрятать из области видимости значения необходимые только для одной из секций.
OutputReport transformReport(final InputReport input) {
final OutputReport output = new OutputReport();
{ //transform report header:
final boolean someFlagUsedOnlyHere = …
… 50 lines of code …
}
{ //transform report body:
… 50 lines of code …
{ //transform section 01:
… 50 lines of code …
}
…
{ //transform section 04 (end section):
… 50 lines of code …
}
}
{ //transform report summary:
… 50 lines of code …
}
return output.
} Конечно, это не всегда лучший подход. Бывает что сколько {} скобок ни добавляй, код лучше не становится, и его просто нужно переписывать. Бывает, что все делится на методы вполне просто и замечательно. Тем не менее, использование {} позволяет добавить выразительности коду там, где это необходимо.
Заключение
Любое приложение, каким бы большим или маленьким оно ни было, в конечном счете состоит из простых вещей: условий, циклов, выражений, моделей данных, в конечном счете строк кода. Как бы красиво ни называлась архитектура приложения, какие бы паттерны программирования и фреймворки в нем ни использовались, главное — то, насколько просто или сложно понять, как работает код, который перед тобой на экране. Приведенные выше примеры позволяют сделать код чуть менее сложным и чуть более выразительным. По крайней мере, для нас они полезны.
Комментарии (33)
qasta
22.02.2018 16:29+2try(final ZipInputStream zipInputStream = new ZipInputStream( new FileInputStream(file), charset)) {...}
В коде выше есть неочевидная ошибка, из за которой FileInputStream может быть не закрыт.
Пожалуйста, поясните свою мысль. Я вижу, что тут стрим zipInputStream закроется обязательно. А он в свою очередь (по правилам упаковки стримов друг в друга) должен закрыть нижележащий FileInputStream.dm_wrike Автор
22.02.2018 16:34-2Ошибка то неочевидная ;)
habradante
22.02.2018 16:55+1Несмотря на документацию, которая говорит о необходимости ручного закрытия потоков связанных с IO, FileInputStream реализует интерфейс AutoCloseable. Так что я тоже присоединяюсь к просьбе пояснить мысль.
dm_wrike Автор
23.02.2018 18:01Мысль в том что Java разработчики не умею решать задачи управления ресурсами,
дело не в том что задача сложная, но в том что в Java с такой проблемой приходится
сталкиваться слишком редко. Необходимый навык не вырабатывается.
Уверен, пытливый читатель может найти ошибку в приведенном мною примере
за пять, максимум десять минут. А когда суть ошибки становится ясна,
возникают логичные вопросы. Почему вопиющая ошибка не была видна сразу?
Что сделать для того чтобы подобные ошибки не повторялись в дальнейшем?
Остальное есть в статье, не буду повторяться.
webkumo
22.02.2018 16:35+1Единственное, что я тут вижу ошибка может произойти в конструкторе ZipInputStream. В частности если charset строка, то может выскочить CharsetUnknown или как-то похоже называющееся исключение… и FileInputStream в такой ситуации не закроется…
Меня вот только смущает, что следующий их пример проблему-то и не решает, ибо там банально один стрим.
PS а с inner-классами вообще беда-беда… они отвратительно влияют на читабельность родительского класса и допустимы в основном тогда, когда являются банальным представлением данных без средств обработки этих данных. Ну т.е. конструктор и геттеры… ну и опционально — сеттеры.
qasta
22.02.2018 16:55Спасибо. То есть предполагается, что ZipInputStream считает заголовок или вроде того? Но конструктор не бросает таких исключений и в качестве charset ожидает объект Charset, а не строку — CharserUncknown тоже не выбросит. Вот если мы туда null передадим — то вылетит NullPointerException — тогда уже файл действительно не закроется. То есть направление вы абсолютно верное указали.
dm_wrike Автор
22.02.2018 18:25PS твратительно влияют на читабельность родительского класса — на самом деле это дело привычки.
lastrix
22.02.2018 19:49PS а с inner-классами вообще беда-беда… они отвратительно влияют на читабельность родительского класса и допустимы в основном тогда, когда являются банальным представлением данных без средств обработки этих данных. Ну т.е. конструктор и геттеры… ну и опционально — сеттеры.
Почитайте что такое method-object.
У вас 10 параметров у функции, 9 из них всегда константы, а функцию вы вызываете много раз в цикле.
Вынести в вложенный класс и у вас будет большой конструктор и вызов метода с одним параметром. Это экономит стек.webkumo
23.02.2018 01:17А пример кода под вашу ситуацию существует? У меня что-то не получается представить себе такую ситуацию без существенного code smell...
И да, для невнимательных, обращаю внимание, что я совсем противник вложенных классов, обладающих действием. Возможность создания "внутреннего представления" я как бы и не отметаю совсем, хотя в большинстве случаев что-то там не так.
lastrix
23.02.2018 06:31
Валидатору требуется для проверки NamedValue 4 обязательных константных аргумента и доступ к данным, класса SequenceType.
Предлагаете, что лучше устанавливать внешний валидатор, когда процедура проверки одна и не меняется? И в принципе не может изменится?
С method-object получается более компактный код.
Есть другой вариант парсер конфигураций 1С. В зависимости от типа объекта в xml, необходимо выполнять свою логику, причем она отличается сильно, но общего очень много.
у базового класса тонна информации, вроде около 10 полей (как констант, так и изменяемых), все необходимы при анализе. Часть создается в процессе анализа.
Раньше весь код представлял из себя кашу разных методов с 5-6 аргументами как минимум, сейчас 3 аргумента максимум и весь код разбит на функциональные блоки — вот конфигурируется документ, вот загружается контент из xml, тут определяются типы.
Про быстродействие даже говорить не хочется, оно просто в разы выше.
Хотя важнее именно повышение читаемости кода.
Или лучше иметь один класс (который в принципе нельзя разбить на независимые друг от друга классы без создания нереального количества мусорного кода), и 100+ методов в нем?
Если не страшно, вот пример из реального, работающего проекта, в исходном виде 1500 строк:
Часть классаfinal class MetaDataMapper { /** Огромное количество констант **/ MetaDataMapper( VirtualFile file, 1СDocument sdk, 1СDocument target, @Nullable Element root ) { this.file = file; this.sdk = sdk; this.target = target; this.root = root; resolver = new OneCv8ClassResolver( target ); sourceFile = new SourceFileStub( file.getAbsoluteName().substring( 1 ), null ); dummyDeclareStatement = DeclareStatementModifier.dummy( file ); } private final VirtualFile file; private final SourceFileStub sourceFile; private final 1СDocument sdk; private final Element root; private final 1СDocument target; private final OneCv8ClassResolver resolver; private final DeclareStatementModifier dummyDeclareStatement; private ClassBuilder classBuilder; private List<ClassBuilder> transformationClasses; void mapElement() { if( file.getName().startsWith( "Subsystem." ) || file.getName().startsWith( "WSReference." ) || file.getName().startsWith( "CommonAttribute." ) ) return; if( root.getTagName().equalsIgnoreCase( "Configuration" ) ) { target.newClass( "System.Конфигурация.ЭтаКонфигурация" ) .parent( TYPE_CONFIGURATION ) .superClass( TYPE_CONFIGURATION.classReference() ) .declareStatement( asStatement( root ) ) .customData( ATTR_UUID, root.getAttribute( ATTR_UUID ) ) .build(); return; } Element properties = XmlUtils.getSingleElementOrDie( root, TAG_PROPERTIES ); Element innerInfo = XmlUtils.getSingleElementOrNull( root, TAG_INNER_INFO ); Element internalInfo = XmlUtils.getSingleElementOrNull( root, TAG_INTERNAL_INFO ); Element childObjects = XmlUtils.getSingleElementOrNull( root, TAG_CHILD_OBJECTS ); Element standardAttributes = XmlUtils.getSingleElementOrNull( properties, TAG_STANDARD_ATTRIBUTES ); resolveThisClass(); transformationClasses = new ArrayList<>(); if( innerInfo != null || internalInfo != null ) new ClassTransformer( innerInfo, internalInfo ).doTransform(); if( childObjects != null ) readChildren( childObjects, standardAttributes, classBuilder, transformationClasses ); if( properties != null && root.getTagName().equalsIgnoreCase( "Document" ) ) registerDocumentSpecials( properties ); notifyComplete(); } void mapFormElement() { if( !root.getTagName().equalsIgnoreCase( "Form" ) ) throw new IllegalStateException(); resolveThisClass(); FormAttributeReader attributeReader = new FormAttributeReader( classBuilder, transformationClasses ); readFormAttrs( attributeReader, TAG_ATTRIBUTES, TAG_ATTRIBUTE ); readFormAttrs( attributeReader, TAG_PARAMETERS, TAG_PARAMETER ); Element elements = XmlUtils.getSingleElementOrNull( root, TAG_ELEMENTS ); if( elements != null ) new FormElementsReader( elements ).read(); Element commands = XmlUtils.getSingleElementOrNull( root, "Commands" ); if( commands != null ) new FormCommandsReader( XmlUtils.getChildren( commands, "Command" ), asStatement( commands ) ).read(); notifyComplete(); } void mapPredefined() { if( !root.getTagName().equalsIgnoreCase( "predefinedData" ) ) throw new IllegalStateException(); resolveThisClass(); for( Element item : XmlUtils.getChildren( root, "item" ) ) { String name = XmlUtils.getTextContentOrDie( item, "name" ); Element codeElement = XmlUtils.getSingleElementOrNull( item, "code" ); String attrType = null; if( codeElement != null ) attrType = codeElement.getAttribute( "xsi:type" ); 1CType type = StringUtils.isBlank( attrType ) ? getFieldType( item, "type" ) : resolver.resolveType( Collections.singletonList( attrType ) ); classBuilder.field( name ).type( type ).declareStatement( asStatement( item ) ).build(); } notifyComplete(); } void mapTxtFile() { 1CClassReference reference = resolver.buildClassStructure( file.getName() ); if( reference == null ) throw new IllegalStateException( "Unable to resolve class: " + file ); classBuilder = target .editClass( reference ) .modifier( dummyDeclareStatement ); transformationClasses = new ArrayList<>(); new PlainClassTransformer().doTransform(); notifyComplete(); } private void resolveThisClass() { 1CClassReference reference = resolver.buildClassStructure( file.getName() ); if( reference == null ) throw new IllegalStateException( "Unable to resolve class: " + file.getName() ); classBuilder = target.editClass( reference ); Element properties = XmlUtils.getSingleElementOrNull( root, TAG_PROPERTIES ); if( !root.getTagName().equals( "predefinedData" ) ) classBuilder.declareStatement( asStatement( root ) ) .customData( ATTR_UUID, root.getAttribute( ATTR_UUID ) ); String tagName = root.getTagName(); if( FORMS.contains( tagName.toLowerCase() ) && isManagedForm() ) { classBuilder.parent( TYPE_MANAGED_FORM ) .modifier( Modifiers.ENFORCE_PARENT_CLASS ); } else if( properties != null && tagName.equalsIgnoreCase( "Constant" ) ) registerConstantValueField( properties ); else if( properties != null && tagName.equalsIgnoreCase( "SessionParameter" ) ) registerSessionParameter( properties ); } private void notifyComplete() { classBuilder.build(); if( transformationClasses != null ) transformationClasses.forEach( ClassBuilder:: build ); } private void registerDocumentSpecials( Node properties ) { ClassBuilder newRegisterRecordsCollection = createRecordsCollection(); createSpecialFields( newRegisterRecordsCollection ); Element registerRecords = XmlUtils.getSingleElementOrNull( properties, "RegisterRecords" ); if( registerRecords != null ) for( Element element : XmlUtils.getChildren( registerRecords, "xr:item" ) ) registerDocumentSpecialMember( newRegisterRecordsCollection, element ); newRegisterRecordsCollection.build(); } private void registerDocumentSpecialMember( ClassBuilder newRegisterRecordsCollection, Node element ) { String typeName = element.getTextContent(); 1CClassReference reference = resolver.buildClassStructure( typeName ); 1CClass aClass = reference == null ? null : target.getClassOrNull( reference ); if( aClass == null ) log.warn( "Failed to resolve: " + typeName ); else { String name = aClass.getName(); String memberTypeName = OneCv8Types.NAMESPACE + '.' + aClass.getParentClass().classReference().getName() + "НаборЗаписей." + name; 1CType memberType = OneCv8Types.parse( memberTypeName ); newRegisterRecordsCollection .field( name ) .type( memberType ) .declareStatement( asStatement( element ) ) .buildField(); } } private void createSpecialFields( ClassBuilder newRegisterRecordsCollection ) { 1CType type = newRegisterRecordsCollection.getReference().to1CType(); for( ClassBuilder aClass : transformationClasses ) if( REF_DOCUMENT_OBJECT.equals( aClass.getSuperClass() ) ) aClass .field( "Движения" ) .type( type ) .declareStatement( aClass.getDeclareStatement() ) .build(); } @NotNull private ClassBuilder createRecordsCollection() { 1CClassReference newRegisterRecordsCollectionReference = OneCv8Language.reference( REF_RECORDS_COLLECTION.getFullName() + '.' + classBuilder.getReference().getName() ); if( target.getClassOrNull( newRegisterRecordsCollectionReference ) == null ) return target.newClass( newRegisterRecordsCollectionReference ) .superClass( REF_RECORDS_COLLECTION ) .parent( REF_RECORDS_COLLECTION.to1CType() ); return target.editClass( newRegisterRecordsCollectionReference ); } private void registerSessionParameter( Node properties ) { ClassBuilder paramClassBuilder = target.editClass( REF_SESSION_PARAMETERS ); String name = Utils.getObjectName( properties ); 1CType type = getFieldType( properties, TAG_TYPE ); paramClassBuilder .field( name ).type( type ).declareStatement( asStatement( root ) ).build() .build(); } private void registerConstantValueField( Node properties ) { 1CType fieldType = getFieldType( properties, TAG_TYPE ); classBuilder.field( "Значение" ) .type( fieldType ) .declareStatement( asStatement( root ) ) .build(); } private boolean isManagedForm() { Element properties = XmlUtils.getSingleElementOrNull( root, TAG_PROPERTIES ); String formType = properties == null ? null : XmlUtils.getTextContentOrNull( properties, "FormType" ); return "managed".equalsIgnoreCase( formType ); } private void readFormAttrs( FormAttributeReader attributeReader, String tagName, String childTagName ) { Element attributes = XmlUtils.getSingleElementOrNull( root, tagName ); if( attributes != null ) for( Element element : XmlUtils.getChildren( attributes, childTagName ) ) attributeReader.read( element ); } private void readChildren( Node childObjects, @Nullable Node standardAttributes, ClassBuilder primaryClass, List<ClassBuilder> secondaryClasses ) { AbstractChildReader classFieldReader = new ClassFieldReader( primaryClass, secondaryClasses ); AbstractChildReader tabularSectionReader = new TabularSectionReader( primaryClass, secondaryClasses ); AbstractChildReader commandReader = new CommandReader( primaryClass, secondaryClasses ); Collection<Element> children = XmlUtils.getChildren( childObjects ); for( Element child : children ) { String tagName = child.getTagName(); if( ALLOWED_CLASS_FIELDS.contains( tagName.toLowerCase() ) ) classFieldReader.read( child ); else if( TAG_TABULAR_SECTION.equalsIgnoreCase( tagName ) ) tabularSectionReader.read( child ); else if( TAG_COMMAND.equalsIgnoreCase( tagName ) ) commandReader.read( child ); else if( !SKIPPED_CLASS_FIELDS.contains( tagName.toLowerCase() ) ) log.error( "Skipping child object of unknown type - " + tagName ); } if( standardAttributes != null ) { Collection<Element> attributes = XmlUtils.getChildren( standardAttributes ); StandardAttributeReader reader = new StandardAttributeReader( primaryClass, secondaryClasses ); attributes.forEach( reader:: readAttribute ); } } private 1CType getFieldType( Node properties, String typeTagName ) private IStatement asStatement( Node node ) private final class FormElementsReader { private final Element elements; private final ClassBuilder elementsSubClass; private final 1CClassReference parentClassReference; private final Deque<1CClassReference> eventTypeSource = new LinkedList<>(); void read() void read( Node elementsNode ) } private final class FormCommandsReader { private final List<Element> commands; private final ClassBuilder formCommands; void read() } private final class FormAttributeReader extends AbstractChildReader { @Override void read( Element element ) } private final class SubTableAttributeReader extends AbstractChildReader { private final String className; private final 1CType parentClass; private final 1CClassReference pathToColumns; @Override void read( Element element ) } private final class StandardAttributeReader { private static final String CUSTOM_DATA_KEY_ALIAS = "alias"; private final ClassBuilder primaryClass; private final List<ClassBuilder> secondaryClasses; void readAttribute( Element attribute ) } private static final class AttributeTemplate { private String name; private 1CType type; private final Collection<Modifier> modifiers = new HashSet<>(); void addModifier( Modifier modifier ) void setType( 1CType type ) public void setName( String name ) String getName() 1CType getType() Collection<Modifier> getModifiers() } private abstract class AbstractChildReader { private final ClassBuilder primaryClass; private final Collection<ClassBuilder> secondaryClasses; ClassBuilder getPrimaryClass() Collection<ClassBuilder> getSecondaryClasses() 1CType getXmlFieldType( Node properties, boolean useClassType, String typeTagName ) abstract void read( Element element ); void forEachClass( Consumer<ClassBuilder> callback ) } private final class ClassFieldReader extends AbstractChildReader { @Override void read( Element element ) } private final class TabularSectionReader extends AbstractChildReader { @Override void read( Element element ) } private final class CommandReader extends AbstractChildReader { @Override void read( Element element ) } private final class ParentMembersReplacer { private final ClassBuilder currentClass; private final Collection<1CClassReference> typesToReplace; private final 1CClass parent; private boolean typeReplaced; void copyMembersFromParent() } private class PlainClassTransformer { private ClassBuilder current; void doTransform() void doPrimaryClassTransformations() void produceType( String name, @Nullable String uuid, Modifier declareStatementModifier ) void registerField( 1CClassReference classReference ) void registerSubClass( String subClassParentName ) } private final class ClassTransformer extends PlainClassTransformer { private final Element innerInfo; private final Element internalInfo; @Override void doTransform() } }
shvilek
22.02.2018 16:49Аналогично в языке Go, все определения по умолчанию являются константами:
greeting := “Hello”
Для определения переменной нужно опять же указать дополнительное слово:
var answer := “Hi”
tour.golang.org/basics/10
Краткая форма объявления переменных
Внутри функции, краткий оператор присваивания := с неявным типом может быть использован вместо объявления с помощью var
fogone
22.02.2018 18:19+1isSupportedBrowser = ((Supplier<boolean>)()->{ … }).get();
этим вы предлагаете заменить хорошо названный метод?dm_wrike Автор
22.02.2018 18:32Хороший метод зачем же заменять. А вот если у вас N-цать строк посреди метода вычисляют одно значение, можно и в Supplier завернуть.
fogone
22.02.2018 19:14+1Так почему же из него метод не сделать? если это N строк, которые вычисляют определенное значение и например могут быть протестированы и вообще могут не мозолить глаза своими деталями?
dm_wrike Автор
22.02.2018 19:31+1Возможно вас путает пример с определением типа браузера, поскольку кажется что эта логика может быть переиспользована, подразумевается что это не так и код одноразовый. Возможно не слишком удачный пример.
Тем не менее, вы апеллируете к тому что нужно вынести метод, но пример про другое.
У вас есть переменная, для ее вычисления вам нужно определить несколько дополнительных переменных которые далее вам не нужны.
Можно просто написать вычисления в коде метода, тогда читая дальнейшую логику
вам придется иметь в виду что есть еще дополнительные переменные, а потом окажется
что для дальнейшей логики они не нужны.
Либо, можно спрятать такие вычисления в Supplier, и тем самым показать что
про все промежуточные переменные дальше можно забыть, не только компилятору,
но и тому кто читает код.fogone
23.02.2018 19:15+1кажется что эта логика может быть переиспользована
вы так говорите, как будто логику в методы выносят только для переиспользования
Либо, можно спрятать такие вычисления в Supplier
или намного проще, визуально понятнее и быстрее «спрятать» их в метод. Понимаю, вы подсмотрели этот паттрен в джаваскрипте, но в джаве он слишком громоздкий. В котлине еще как-то можно было бы что-то такое с инлайн-функциями провернуть
и то редко такое может пригодиться.val result = run { "got it" }dm_wrike Автор
24.02.2018 00:14Получается, вы имеете в виду примерно следующее:
... final Тип имяЗначения = вычислитьИмяЗначения(параметр1, параметр2, ...);
Это может быть вполне хорошим способом структурировать код,
однако:
1. Имя метода типично будет дублировать имя значения и не несет нового смысла.
2. Для того чтобы понять как вычисляется значение нужно будет перейти
в тело метода, посмотреть логику там, потом вернуться и продолжить читать
основной метод. Муторно для одноразового метода.
3. Если параметров для вычисления много, одноразовый метод становится
еще и громоздким. Нужно будет не только придумать имена для всех
параметров но и держать их в голове при чтении.
Либо:
final Тип имяЗначения = ((Supplier<Тип>)()->{ … }).get();
vedenin1980
23.02.2018 18:00Вот именно за такие советы как сделать код «проще» и считают Java плохим языком.
1)
new Runnable() {...}.run();
Анонимный класс, да с вызовом сразу после этого метода — тихий ужас. Читая такой код можно голову сломать к чем относится тот или иной метод, как происходит выполнение и т.д. Не говоря уже о избыточности и многословности.
2)
final boolean isSupportedBrowser = ((Supplier<Boolean>)()->{...}).get();
Бррр. Простая функция в разы проще и понятнее этих безумных скобок, приведений и get'ов. Да, может быть приватная функция, которая используется только один раз, только чтобы декомпозировать код (см. «чистый код»), это намного лучше чем подобное. Явный пример попытки забивать гвозди микроскопом.
Читать такой код сложно, не сразу понятно, что там идет получения значение, начинающий разработчик сломает голов пытаясь понять такой код.
3)
{ //transform report header: final boolean someFlagUsedOnlyHere = … … 50 lines of code … }
Если у вас возникает такая потребность значит у вас что-то с кодом неправильно, стоит выделить функции или внутренние классы и работать с ними. Функция с правильным названием намного лучше такого блока с комментарием. Практически всегда это можно сделать.
4)
При этом аккуратное решение совсем простое:
public class MathExt { public static class SumAndAverage { private final long sum; private final double average; public SumAndAverage(final long sum, final double average) {...} public long getSum() {...} public double getAverage() {...} } public static SumAndAverage computeSumAndAverage(final ImmutableList<Integer> valueList) { ... } }
Иногда это имеет смысл, но в большинстве случаев, если SumAndAverage будет часто использоваться в других классах, у вас будут либо статические импорты (что не очень рекомендуется), либо некрасивый и длинный код MathExt.SumAndAverage во многих местах. В большинстве случаев лучше либо использовать Pair или его аналог, если это временные объекты, которые сразу будут удалены после получения, либо делать SumAndAverage отдельным DTO классом.
Если значения использовать и присваивать сразу, то разницы практически нет:
MathExt.SumAndAverage sumAndAverage =…
int sum = sumAndAverage.getSum();
double average = sumAndAverage.getAverage();
…
И
Pair<Integer,Double> sumAndAverage =…
int sum = sumAndAverage.geFirst();
double average = sumAndAverage.getSecond();
…
Мы и так видим, что это сумма и среднее. А на генерацию и сопровождение такого количества внутренних классов тратится время и ухудшается читабельность самого класса.dm_wrike Автор
23.02.2018 18:44Хотя в нашем подходе нет ничего экстраординарного, он во многом отличается от того, как принято писать код на Java. Тем не менее, кто-то может найти нечто полезное в нашем подходе и для себя.
Обратите внимание, я нигде не утверждаю что приведенные мною техники написания кода
являются единственно правильными. Наоборот, я подчеркиваю что они скорее отличаются
от общепринятого подхода к написанию Java кода.
Каждый разработчик при написании кода использует определенные приемы,
и код написанный подобным образом ему, само собой, кажется простым и понятным.
Приведенные мною примеры не являются типичными, так что на первый взгляд может
показаться что они хуже читаются.
Приведенные выше примеры позволяют сделать код чуть менее сложным и чуть более выразительным. По крайней мере, для нас они полезны.
Надеюсь что они окажутся полезны и для кого то еще.
Но для этого нужно изменить некоторые привычки написания и чтения кода.
Это сложно, и вообще говоря, не обязательно.
relgames
24.02.2018 17:06Pair нарушает инкапсуляцию. Это знание, какое поле что означает, все равно есть, только в неявном виде. Гораздо более правильно (true OOP ;) ) это знание определить явно, в виде класса. Скорее всего, оно все скаляризуется и класса там не будет вообще. Для уменьшения кода мы используем Lombok.
dimaaan
В C# есть кортежи:
dm_wrike Автор
Предложение было про определение нескольких классов в одном файле.
Кортежи это хорошо, иногда, но в Java их нет.
sshikov
Это отчего-то их нет? Pair или Tuple, от которого вы почему-то воротите нос, в общем-то и является кортежем. Вполне статически типизированным, и удобным в работе. А вот тот именованный класс, который вы вместо предлагаете, лично у меня как раз вызывает отторжение — потому что он как раз не может быть как правило повторно использован.
vooft
Pair/Tuple не дают никакой информации о том, что же там за данные. Вот функция doOperation() возвращает Pair<String, Integer> — что это за данные? Без документации не разобраться.
Если вернуть класс с полями типа String errorMessage, Integer errorCode — сразу понятно, что же это за зверь.
sshikov
А они и не должны. Их задача — сообщить тип компонентов пары. Если вам нужно чтобы сообщение об ошибке было не просто String — вот для него и заведите осмысленный класс (которых у вас будет N, а не N*M, как в случае для бессмысленных классов на каждую пару типов). Или даже не заводите — а верните скажем Pair<Integer, Exception>, где будет ваше сообщение об ошибке, стек и еще что-то полезное.
sshikov
И я попрошу заметить, что изначально просили кортежи. Если вы откроете определение кортежа, то увидите, что там нет никаких имен. Поэтому то что вы просите — это не кортеж.
ApeCoder
Как это так, C# — клон Java, а кортежей в ней нет? ;)
playermet
Да есть они там, это уже упомянутый выше Tuple.