Войти
ФлеймФорумПрограммирование

Оцените кусочек кода из google/protobuf/compiler/parser.cc

Страницы: 1 2 3 4 Следующая »
#0
2:25, 26 окт. 2018

Всем привет.

https://github.com/protocolbuffers/protobuf/blob/0038ff49af882463… arser.cc#L758

bool Parser::ParseMessageStatement(DescriptorProto* message,
                                   const LocationRecorder& message_location,
                                   const FileDescriptorProto* containing_file) {
  if (TryConsumeEndOfDeclaration(";", NULL)) {
    // empty statement; ignore
    return true;
  } else if (LookingAt("message")) {
    LocationRecorder location(message_location,
                              DescriptorProto::kNestedTypeFieldNumber,
                              message->nested_type_size());
    return ParseMessageDefinition(message->add_nested_type(), location,
                                  containing_file);
  } else if (LookingAt("enum")) {
    LocationRecorder location(message_location,
                              DescriptorProto::kEnumTypeFieldNumber,
                              message->enum_type_size());
    return ParseEnumDefinition(message->add_enum_type(), location,
                               containing_file);
  } else if (LookingAt("extensions")) {
    LocationRecorder location(message_location,
                              DescriptorProto::kExtensionRangeFieldNumber);
    return ParseExtensions(message, location, containing_file);
  } else if (LookingAt("reserved")) {
    return ParseReserved(message, message_location);
  } else if (LookingAt("extend")) {
    LocationRecorder location(message_location,
                              DescriptorProto::kExtensionFieldNumber);
    return ParseExtend(message->mutable_extension(),
                       message->mutable_nested_type(),
                       message_location,
                       DescriptorProto::kNestedTypeFieldNumber,
                       location, containing_file);
  } else if (LookingAt("option")) {
    LocationRecorder location(message_location,
                              DescriptorProto::kOptionsFieldNumber);
    return ParseOption(message->mutable_options(), location,
                       containing_file, OPTION_STATEMENT);
  } else if (LookingAt("oneof")) {
    int oneof_index = message->oneof_decl_size();
    LocationRecorder oneof_location(message_location,
                                    DescriptorProto::kOneofDeclFieldNumber,
                                    oneof_index);

    return ParseOneof(message->add_oneof_decl(), message,
                      oneof_index, oneof_location, message_location,
                      containing_file);
  } else {
    LocationRecorder location(message_location,
                              DescriptorProto::kFieldFieldNumber,
                              message->field_size());
    return ParseMessageField(message->add_field(),
                             message->mutable_nested_type(),
                             message_location,
                             DescriptorProto::kNestedTypeFieldNumber,
                             location,
                             containing_file);
  }
}
Если не обращать внимание на форматирование и стиль(цепочка "else if"), то как вам: норм или уг?


#1
(Правка: 2:54) 2:50, 26 окт. 2018

Adler
> Если не обращать внимание на форматирование и стиль
не получилось не обращать - глаза от такого форматирования вытекли раньше чем я понял что там написано.

Не ожидал такого от гугловского кода


Например с какого перепугу команды else if и return из них на одном уровне? Это уже создает путаницу - при беглом листании непонятно откуда это return летит

 } else if (LookingAt("reserved")) {
   return ParseReserved(message, message_location);
 } else if (LookingAt("extend")) {


Теперь понятно почему у гугла софт такой убогий

#2
(Правка: 6:18) 6:11, 26 окт. 2018

типичный код для парсера. стиль выдержан.

может быть и можно как-то это else if цепочки оптимизировать, но зачем? сильно скорости прибавит?

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

я больше видел обратных примеров, когда (рекурсивный) парсер наткнувшись на какой-нить хлам, быстренько делает что-то вроде эксепшена (в языках где есть исключения -> исключения, где исключений нет - какой-нить джамп наружу с зачисткой стэка), дабы не возиться с проверками успешности.

#3
(Правка: 7:11) 7:05, 26 окт. 2018

Выравнивание черезжепное, else в одной строчке с закрывающей скобкой блока if, аргументы в отдельных строках при том, что первый из них в одной строке с вызовом функции... мде.
В общем обычный говнокод.

Вдогонку: Иногда на одной строчке с вызовом функции всего один аргумент, а иногда - два. Низачот.
const использовать забывают, да еще и строки в константы не вынесены.

#4
7:11, 26 окт. 2018
+ Показать
#5
7:16, 26 окт. 2018
+ Я сделяль
#6
(Правка: 7:30) 7:27, 26 окт. 2018

Great V.
> В общем обычный говнокод.
Я бы это даже говнокодом не назвал - говнокод, он от лени и незнания...
А вот так форматировать код - это надо иметь альтернативное мышление

(о, так еще и стиль именования хромает, то верблюд-стиль, то с черточкой... короче, понабрали в этот гугль по объявлению)

#7
7:48, 26 окт. 2018

Great V.
war_zes


tl;dw: то, что вы неосилили, ещё не значит, что код плохой.
#8
8:04, 26 окт. 2018

Delfigamer
> tl;dw: то, что вы неосилили, ещё не значит, что код плохой.
Неосиливание кода из-за альтернативного форматирования - 100% признак что код плохой
Код должен не только выполнять задачу, но и быть читаемым, чтобы эту задачу потом смогли развивать


(а еще в этом коде полно запахов для рефакторинга. Огромный свитч из копипаста - это тоже не признак здорового кода)

#9
8:11, 26 окт. 2018

Delfigamer
> tl;dw: то, что вы неосилили, ещё не значит, что код плохой.
war_zes
> а еще в этом коде полно запахов для рефакторинга. Огромный свитч из копипаста -
> это тоже не признак здорового кода)

А кто вам сказал, что этот код вообще человек написал? Парсеры, обычно, это выхлоп кодогенерации... и читаемость там всегда плохо, ибо оно и не предназначено для чтения/правки.

#10
8:51, 26 окт. 2018

Не знаю, что там делает функция "LookingAt", но использовать её подряд... В общем, пусть лучше дальше свои спортивные задачки решают

#11
9:09, 26 окт. 2018

0iStalker
> Парсеры, обычно, это выхлоп кодогенерации...
Есть такая штука - история изменений файла :) Человек пишет. Да там весь код в таком недостиле
типа
https://github.com/protocolbuffers/protobuf/blob/0038ff49af882463… otobuf/any.cc

#12
9:10, 26 окт. 2018

Ещё один, блин, профессор нарисовался.
Mimon
> Не знаю, что там делает функция "LookingAt", но использовать её подряд...

inline bool Parser::LookingAt(const char* text) {
  return input_->current().text == text;
}
Она делает ровно то, чего от неё хотят, и ровно так, как следует.

#13
9:13, 26 окт. 2018

И эти все профессора как-то дружно упускают тот факт, что парсер - это вещь, которая читает содержимое текстового файла. Нахрена дрочить байты, если всё равно быстрее дискового I/O не убежишь?

#14
(Правка: 9:23) 9:21, 26 окт. 2018

Delfigamer
Ты упускаешь ту вещь, что кроме парсера там и другой, вспомогательный код - и он весь вот такой говняный


p.s. чистый код пишут не ради оптимизации, поэтому твое замечание про дрочку байтов неуместно

Страницы: 1 2 3 4 Следующая »
ФлеймФорумПрограммирование