• ,

Оформление кода

Одну из задач (которая была решена и принята сервером) я реализовал в виде кода (забил * и X-ми чтобы не спойлерить).

/* xxx
1. Считывать строки(параметры) с консоли, пока пользователь не введет пустую строку(Enter).
2. Каждый параметр соответствует имени ***.
Для каждого параметра:
3. Создать объект *** класса ***, который равен *** из getX(String параметр).
4. Вывести на экран toString().
*/

public class Solution
{
    public static void main(String[] args) throws Exception
    {
        //Add your code here
        List<String> al= new ArrayList<>();
        BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));
        String str;
        while(!"".equals(str = reader.readLine())) al.add(str);
        for (int i = 0; i <al.size(); i++) System.out.println(XFactory.getX(al.get(i)).toString());
    }
...

Зашедший в гости полузнакомый раскритиковал, что так не пишут (это тебе не C и т.п.).
Не подскажете в чем я не прав (есть какие нормы и общепринятые стандарты)?
Или использование в одной строчке нескольких конструкций (оно вообще, без промежуточных присваиваний временным переменным, может привести в каких-либо ситуациях к каким-либо возможным последствиям?) — дурной тон?
Основные переменные (al, reader, str) объявлены до их использования (pascal-стиль), так как мне удобнее, всегда знаешь где можешь найти описание переменной с возможным комментарием.
Вспомогательные переменные объявлены в нужной зоне видимости (как переменная цикла i).
Idea не ругается, правда предлагает 2 опции:
Split into declaration and assigment (таки разделить объявление и присваивание) и заключить некоторые участки в блок try.

Как бы Вы правильнее написали код?

Что не так?
Для такой задачи не стоит комментировать каждую строчку.

13 комментариев

yag0andy
Вот такое:

for (int i = 0; i <al.size(); i++) System.out.println(XFactory.getX(al.get(i)).toString());


я предпочел бы заменить на:


for (String line : list) {
    System.out.println(XFactory.getX(line).toString());
}


Названия переменных типа «а1» суть зло ))
Joysi
  • Joysi
  • 0
  • Комментарий отредактирован 2016-01-20 11:36:40 пользователем Joysi
Спасибо, работа через итератор принимается (Я по привычке все через циклы делаю, надо уже переучиваться ведь не для всех типов можно линейным перебором по индексу работать). Там не a1, там — al (aL от ArrayList — обычно для имен переменных от сложных типов первые символы даю соответствующие типу, чтобы сразу понять чьих она будет).
Вопрос — а зачем обрамлять в {} если один оператор? На случай будущих кодовых вставок в тело цикла? Или распространенные сторонние инструменты анализа кода для будущего рефакторинга не распознают?
Но таким образом ведь мы лишаем себя видеть дополнительную пару строк кода в текущей области окна редактора кода.

И еще вопрос (с точки зрения прекомпиляции и используемой памяти): обе конструкции генерируют одинаковый байт-код и одинаково расходуют память?
//1
System.out.println(XFactory.getX(al.get(i)).toString())
//2
str     = al.get(i);
objMeow = XFactory.getX(str);
System.out.println(objMeow.toString());
yag0andy
По поводу {}. Дело вкуса. Я иногда обрамляю, иногда нет. В циклах обрамляю. Не обрамляю в таких случаях:

if (needClose) 
    close();


Такое не грех и в одну строку записать. Т.е. когда конструкция крайне проста.

Переменная al и а1. В некоторых шрифтах разницу не заметить. Я вот не заметил.

Такое
while(!"".equals(str = reader.readLine())) al.add(str);

ну тоже не в одну строчку… и покрасивше… Я такое видел в «решениях», но мне не очень нравится.

ИМХО красивее было бы что-то типа:

String buffer;
while (!(buffer = reader.readLine()).equals("")){
    list.add(buffer); 
}

Но, не настаиваю. Тут всё-таки мы в вторгаемся в тему предпочтений, а тут кому как )
yag0andy

String buffer;
while (!(buffer = reader.readLine()).isEmpty()){
    list.add(buffer); 
}

Нет пределов совершенству
EvIv
while (!(buffer = reader.readLine()).equals(""))

опасно! а если reader.readLine() вдруг вернет null?
Всегда лучше сравнивать строковую переменную с константой так:
"someConstString".equals(varStr)
, тогда гарантированно не поймаете NullPointerException, так как equals() вызовется на точно существующем объекте «someConstantString»
yag0andy
Соглашусь. В этом случае и применение isEmpty() не безопасно.
Joysi
Таки моя конструкция
while(!"".equals(str = reader.readLine())) al.add(str);

имеет больше прав на существование :-)
blacky
Относительно записи нескольких операторов в одну строку (в основном это if, for, while) — есть одно правило, которое мне очень понравилось и смысл его в следующем:
— если результат улучшает общую читабельность кода и меньше 80 символов, то можно записать в одну строку без фигурных скобок;
— если же нужно разместить на нескольких строках, то всегда следует использовать фигурные скобки.
Joysi
Сейчас, через квартал с хвостиком. Я бы переписал свое
while(!"".equals(str = reader.readLine())) al.add(str);

в виде
while(!"".equals(str = reader.readLine()))
    al.add(str);

Из-за отступа сразу видна зависимость оператора (от шапки цикла, условия...)
P.S. Все это субъективно + если работаете в коллективе могут быть наложены и свои правила.
Nezhelskiy
Замечу, что написанный в одну строку блок в режиме отладки будет выполняться целиком и невозможно будет проследить по-шагово, что происходит в цикле, например.
Viktoor_
Запарсил бы reader.readline в int
OleJoMan
public static void main(String[] args) throws Exception
{
//Add your code here
Listal= new ArrayList<>();

BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));

String str = reader.readLine();

while(!str.equals(""))
{
al.add(str);
str = reader.readLine();
}

for (int i = 0; i <al.size(); i++) //Заменил бы на for each
{
System.out.println(XFactory.getX(al.get(i)).toString());
}
Только зарегистрированные и авторизованные пользователи могут оставлять комментарии.