Обход связанного списка, таблица символов компилятора и проверка дубликатов идентификаторов - PullRequest
Новая
       31

Обход связанного списка, таблица символов компилятора и проверка дубликатов идентификаторов

0 голосов
/ 24 февраля 2010

Мой "учебник" выходит:

а) 

Obj p = curScope.locals, last = null;
while (p != null) {
    if (p.name.equals(name)) error(name + " declared twice");
    last = p; p = p.next;
}
if (last == null) curScope.locals = obj; else last.next = obj;

Должен ли я рефакторинг по этим направлениям?

б) 

if (curScope.locals == null) {
  curScope.locals = obj;
} else {
  Obj p = curScope.locals;
  while (true) {
    if (p.name.equals(name))
      error(name + "declared twice");
    if (p.next == null) 
      break;
    p = p.next; // Good catch, @michaelmior
  }
  p.next = obj;
}

Редактировать: удалены длинные имена переменных, поскольку я думал, что они добавят ясности намерений, но они этого не сделали. Спасибо @danben. Очевидно, что чем меньше, тем лучше.

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

Edit:

Мои мысли по поводу б) как «улучшение» звучали так:

  • на одно меньше одноразового имени переменной: p вместо p, last
  • p выходит из области видимости после обхода, идентификатор освобожден
  • на curScope.locals == ноль для начала, более логичный путь выполнения
  • нет необходимости в погоне за указателем, как last = p; p = p.next
  • закрытие присваивания не в if / else

но я не уверен, так как в б):

  • наиболее часто выполняемый путь находится в ветви else

Есть ли лучший в)?

PS: мой первый вопрос, простите за первоначальное замешательство с инструментом, я все еще учусь.

Ответы [ 2 ]

0 голосов
/ 24 февраля 2010

(b) не будет работать, так как вы никогда не обновите traverseTableObjectsLinkedList, и у вас будет бесконечный цикл. Поэтому я предпочитаю (а). Даже если вы это исправите, я не думаю, что на самом деле есть большая разница, и любой выбор между ними можно отнести к личным предпочтениям.

0 голосов
/ 24 февраля 2010

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

Современные компиляторы очень хороши в оптимизации и могут сделать многие микрооптимизации программиста устаревшими. Однако они не могут сделать ваш код более понятным для других. Таким образом, хорошее правило, которому нужно следовать, это в первую очередь написать свой код так, чтобы его было легко читать.

Если после этого вы обнаружите, что ваша программа работает слишком медленно или занимает слишком много памяти, найдите причину этой конкретной проблемы и начните оптимизацию.

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

Obj p = curScope.locals, last = null;
while (p != null) {
    if (p.name.equals(name)) {
        error(name + " declared twice");
        last = p;
        p = p.next;
    }
}
if (last == null) {
    curScope.locals = obj; 
} else {
    last.next = obj;
}

Дополнительные несколько нажатий клавиш избавят вас от головной боли, если вы или кто-либо другой решит добавить инструкцию к одному из этих предложений if или else, не забывая добавить фигурные скобки.

...