4 ms·

This is a good article and I like the general idea it provides. I want to add a small disagreement, though. > With the Refactoring hat , you are not allowed to

This is a good article and I like the general idea it provides. I want to add a small disagreement, though. > With the Refactoring hat , you are not allowed to change the way the code works. This is the only part I have a small disagreement with. Sometimes, the act of refactoring it _does_ change how the code works, but it's a change in how the code handled ambiguous or extremely complicated states. Sometimes refactoring results in streamlining an if-statement / configuration-detail mess and it's the right time to fix it. It may be way, way, way extra work to put all the bugs intentionally back into the code in the refactor, and you may not even put the bugs back in correctly.

nicbou 5y ago
While refactoring, I have to understand what the component does or should do. I read the original code and sometimes the commit history for certain lines. Then I write tests to ensure those assumptions carry to the new code. More than once, I ended up finding bugs in the old code this way. The ticket said one thing (ticket IDs in commit messages are a godsend), and the code did another thing. There was also a lot of unexpected business logic that was discovered after the first draft of the code, and not documented. I had the same experience translating text content into calculators. Text can be ambiguous, but code can't. Text had errors that coded logic wouldn't allow.
Jtsummers 5y ago
In most of the examples of refactoring there's a strong distinction made between changing how something works internally and changing its behavior. The behavior is its external behavior, not internal. As a somewhat trivial example, in Common Lisp you may use an association list like: '((a . b) (c . d) (e . f)) This is grossly inefficient for some operations, but functions as a map. You realize, "Oh, we have alists with, potentially, thousands of elements or have to search it over and over, it's killing our performance". So you replace that with a hash table to get that nice O(1) performance on a lot of your operations. Your users though, never see a behavioral change because they still access the data using the same methods as before: (get-value map 'a) To them, there has been no change, and consequently for behavioral tests there has also been no change. It performs better (so there is a change, obviously) but it does not impact the guarantees you offered users of the unit.
- User23 5y ago
  > This is grossly inefficient for some operations, but functions as a map I believe it's actually faster than a hash table for a decently large n, around the order of 50. Obviously this is going to depend quite a lot on how clever your compiler is. edit: I was off by an order of magnitude, 500 -> 50. I'm not able to find if any of the free Common Lisps do any kind of CDR coding[1]. [1] https://en.wikipedia.org/wiki/CDR_coding https://en.wikipedia.org/wiki/CDR_coding
  - Jtsummers 5y ago
    I've never benchmarked the two as I usually either need something small (so use alists) or of large or indeterminate size (so use hash tables). There's a clear performance difference between the two choices at that second case, but since I never hit the "middle ground" I don't know what the threshold is. It probably also varies with each implementation. Hash tables, though, should be O(1) for retrieval and updates and amortized O(1) for insertions. Alists are O(n) for retrieval and updates and O(1) for insertions (if you don't check for the presence of the key).
  - TeMPOraL 5y ago
    If you want performance, you may be better off using arrays in some cases. Instead of an alist or a hash table, make a struct - or even just a cons cell like: (cons keys values). Then have your association function do a linear search for target key on (car table), e.g. via #'position, and use its index to fetch the value from (cdr table) directly.

nicbou 5y ago

While refactoring, I have to understand what the component does or should do. I read the original code and sometimes the commit history for certain lines. Then I write tests to ensure those assumptions carry to the new code. More than once, I ended up finding bugs in the old code this way. The ticket said one thing (ticket IDs in commit messages are a godsend), and the code did another thing. There was also a lot of unexpected business logic that was discovered after the first draft of the code, and not documented. I had the same experience translating text content into calculators. Text can be ambiguous, but code can't. Text had errors that coded logic wouldn't allow.

Jtsummers 5y ago

In most of the examples of refactoring there's a strong distinction made between changing how something works internally and changing its behavior. The behavior is its external behavior, not internal. As a somewhat trivial example, in Common Lisp you may use an association list like: '((a . b) (c . d) (e . f)) This is grossly inefficient for some operations, but functions as a map. You realize, "Oh, we have alists with, potentially, thousands of elements or have to search it over and over, it's killing our performance". So you replace that with a hash table to get that nice O(1) performance on a lot of your operations. Your users though, never see a behavioral change because they still access the data using the same methods as before: (get-value map 'a) To them, there has been no change, and consequently for behavioral tests there has also been no change. It performs better (so there is a change, obviously) but it does not impact the guarantees you offered users of the unit.

User23 5y ago
> This is grossly inefficient for some operations, but functions as a map I believe it's actually faster than a hash table for a decently large n, around the order of 50. Obviously this is going to depend quite a lot on how clever your compiler is. edit: I was off by an order of magnitude, 500 -> 50. I'm not able to find if any of the free Common Lisps do any kind of CDR coding[1]. [1] https://en.wikipedia.org/wiki/CDR_coding https://en.wikipedia.org/wiki/CDR_coding
- Jtsummers 5y ago
  I've never benchmarked the two as I usually either need something small (so use alists) or of large or indeterminate size (so use hash tables). There's a clear performance difference between the two choices at that second case, but since I never hit the "middle ground" I don't know what the threshold is. It probably also varies with each implementation. Hash tables, though, should be O(1) for retrieval and updates and amortized O(1) for insertions. Alists are O(n) for retrieval and updates and O(1) for insertions (if you don't check for the presence of the key).
- TeMPOraL 5y ago
  If you want performance, you may be better off using arrays in some cases. Instead of an alist or a hash table, make a struct - or even just a cons cell like: (cons keys values). Then have your association function do a linear search for target key on (car table), e.g. via #'position, and use its index to fetch the value from (cdr table) directly.