Refactor of table commands #131
Conversation
| execute(): void { | ||
|
|
||
| const selection= this.model.selection; | ||
| execute(above: boolean, selection: ModelSelection = this.model.selection) { |
There was a problem hiding this comment.
using a boolean to determine behavior is a bit of a code smell IMHO. I find that usually it's better to have two differently named methods in such cases. In this case I think that translates here into having two separate commands. We can still use abstractions to avoid code duplication, but giving the specific behavior its own name makes it easier to look for the specific usage later on. It's usually quite hard to search for a method with a certain param value (though some IDE's can do that for you)
There was a problem hiding this comment.
That's a good point. I wasn't too sure about this either. I gave the insert column before and after + row above and below their own classes again, that make use of the abstraction made earlier.
Some small changes to commands that work on tables.
Combine insert row above and below + insert column before and after.