Skip to main content
added 274 characters in body
Source Link
JacquesB
  • 62.4k
  • 21
  • 137
  • 190

For what its worth, I also find your changed code quite confusing compared to the original code.

The original code is beautifully simple. Apart from the interface declaration it is basically a three-way switch. Your code is a lot more complex including a factory, a builder pattern and something called a randomIdentifier (WTF?). Complexity leads to bugs and code that is hard to change, so it is the enemy of maintenance. If I had to review this code as pull request, my first question is what real-world problem the old code has which justifies this manifold increase in complexity?

The problem is not that I don't understand the patterns in use. The problem is I don't understand why you need all these patterns and complexity. To put it another way, it is not that any particular part of the code is especially confusing, it is more that I don't understand why you think you need all this complexity in the first place.

So I think you should focus more on explaining why you write the code you do, rather than how the code works. Show some concrete problem with the existing code which everybody will agree is a problem. For example that you use a lot of time adding new properties or that you often have bugs where actions are added incorrectly. Then explain why your code makes these problems go away.

For what its worth, I also find your changed code quite confusing compared to the original code.

The original code is beautifully simple. Apart from the interface declaration it is basically a three-way switch. Your code is a lot more complex including a factory, a builder pattern and something called a randomIdentifier (WTF?). Complexity leads to bugs and code that is hard to change, so it is the enemy of maintenance. If I had to review this code as pull request, my first question is what real-world problem the old code has which justifies this manifold increase in complexity?

The problem is not that I don't understand the patterns in use. The problem is I don't understand why you need all these patterns and complexity. To put it another way, it is not that any particular part of the code is especially confusing, it is more that I don't understand why you think you need all this complexity.

So I think you should focus more on explaining why you write the code you do, rather than how the code works.

For what its worth, I also find your changed code quite confusing compared to the original code.

The original code is beautifully simple. Apart from the interface declaration it is basically a three-way switch. Your code is a lot more complex including a factory, a builder pattern and something called a randomIdentifier (WTF?). Complexity leads to bugs and code that is hard to change, so it is the enemy of maintenance. If I had to review this code as pull request, my first question is what real-world problem the old code has which justifies this manifold increase in complexity?

The problem is not that I don't understand the patterns in use. The problem is I don't understand why you need all these patterns. To put it another way, it is not that any particular part of the code is especially confusing, it is more that I don't understand why you think you need all this complexity in the first place.

So I think you should focus more on explaining why you write the code you do, rather than how the code works. Show some concrete problem with the existing code which everybody will agree is a problem. For example that you use a lot of time adding new properties or that you often have bugs where actions are added incorrectly. Then explain why your code makes these problems go away.

Source Link
JacquesB
  • 62.4k
  • 21
  • 137
  • 190

For what its worth, I also find your changed code quite confusing compared to the original code.

The original code is beautifully simple. Apart from the interface declaration it is basically a three-way switch. Your code is a lot more complex including a factory, a builder pattern and something called a randomIdentifier (WTF?). Complexity leads to bugs and code that is hard to change, so it is the enemy of maintenance. If I had to review this code as pull request, my first question is what real-world problem the old code has which justifies this manifold increase in complexity?

The problem is not that I don't understand the patterns in use. The problem is I don't understand why you need all these patterns and complexity. To put it another way, it is not that any particular part of the code is especially confusing, it is more that I don't understand why you think you need all this complexity.

So I think you should focus more on explaining why you write the code you do, rather than how the code works.