Project

General

Profile

Actions
Copy link

Task #5039

closed

[verilog][parser][cfg] Неправильное оформление кода

Added by Alexander Kamkin over 10 years ago. Updated over 10 years ago.

Status:
Closed
Priority:
High
Assignee:
Mikhail Chupilko
Category:
-
Target version:
0.1
Start date:
07/09/2014
Due date:
% Done:

0%

Estimated time:
Detected in build:
svn
Published in build:

Description

  • Оформление кода не соответствует соглашениям Google (настройки есть в проекте - eclipse-java-google-style.xml).
  • Вместо пробелов встречаются символы табуляции.
  • Отсутствует единообразие в именовании полей (conditionBlock, cfgtoremove -> cfgToRemove; tmp, temp).
  • Нет коментариев - коментарии к полям, методам и к коду внутри методов нужно писать сразу.
  • Много лишнего кода - методы принтера.
  • ru.ispras.verilog.parser.model.Activity.Type => Activity.Type.
  • Вместо System.err.println("MACROMODULE is not supported") нужно использовать функции логирования.

P.S. В целом код небрежный.

Actions
Copy link
 #1

Updated by Mikhail Chupilko over 10 years ago

  • Status changed from New to Open

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

Actions
Copy link
 #2

Updated by Mikhail Chupilko over 10 years ago

  • Status changed from Open to Resolved

Сделано все кроме методов принтера, которые исчезнут по окончании разработки парсера.

Actions
Copy link
 #3

Updated by Alexander Kamkin over 10 years ago

  • Status changed from Resolved to Open
  1. Соглашения Google не выполняются (скобки должны быть на той же строке, между if, while, for и условием должен быть пробел).
  2. Комментариев мало (даже шапки в некоторых файлах отсутвуют).
  3. Alexander Kamkin -> Mikhail Chupilko.
  4. Встречается лишний код (например, Storage - это обычный стек; в Java есть класс java.util.Stack).
  5. Имена типа arg0 - плохая практика.
  6. Комментарии "// TODO Auto-generated method stub" должны быть удалены.
  7. Классы NodeVisitor и VerilogNodeParser лучше сделать внутренними классами VerilogCfgBuilder.
  8. Классы NodeVisitor и VerilogNodeParser лучше переименовать в ExprVisitor и VerilogVisitor.
  9. Встречается странный код, например,
    public final class VerilogCfgBuilder extends VerilogBackend {
  protected CfgModel cfgModel; // Какой смысл в protected-поле в final-классе?
}

или

public Result onPortConnectionBegin(final PortConnection node) {
    ...
    NodeVariable var = new NodeVariable(new Variable(portName, DataType.BIT_VECTOR(64)));
    var.setUserData(new VariableData(VariableType.INOUT));

    // Переменная var нигде не используется!

Это только примеры.

Actions
Copy link
 #4

Updated by Alexander Kamkin over 10 years ago

Предлагаю при создании строк из нескольких кусочков использовать метод format:

AS IS:

"Variable " + rangedVariable.getVariable().getName() + " is not a variable in module " + getCurrentModule().getName() +"!"

TO BE:

String.format("Variable '%s' is not a variable in module '%s'!", rangedVariable.getVariable().getName(), getCurrentModule().getName());

Actions
Copy link
 #5

Updated by Mikhail Chupilko over 10 years ago

  • Status changed from Open to Resolved

Комментарии добавляются по ходу пьесы (а так, вообще, они дополнены), остальное - сделано.

Actions
Copy link
 #6

Updated by Alexander Kamkin over 10 years ago

  • Status changed from Resolved to Closed
Actions
Copy link

Also available in: Atom PDF