My "textbook" goes:

a)

```
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;

```

Should I refactor along these lines?

b)

```
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;
}

```

Edit: removed long variable names as I thought they would add clarity of intent but they did not. Thanks @danben. It's obvious that less is more.

Edit: (as I cannot comment back) yes, I should have used the same variable names in the first place. Sorry about that.

Edit:

My thoughts on b) as an "improvement" were:

* one less disposable variable name: p instead of p, last
* p goes out of scope after traversal, identifier freed
* on curScope.locals == null to begin with, more logical execution path
* no need for pointer chasing like last = p; p = p.next
* closing assignment not under an if/else

but I am unsure since in b):

* the most frequently executed path is actually under an else branch

Is there a better c) ?

PS: my first question, forgive my initial confusion with the tool, I am still learning.