ngx-drag-to-select
ngx-drag-to-select copied to clipboard
proposal: refactor conditionals
Hi,
an idea could be to refactor the conditionals into certain rule lists.
Disclaimer
🤗 This issue is just an idea according to https://twitter.com/elmd_/status/1181875171667451905.
Propsal
const addRules = [
withinBoundingBox && !item.selected && this.selectMode,
withinBoundingBox && !this.shortcuts.toggleSingleItem(event) && !this.selectMode && !this.selectWithShortcut,
withinBoundingBox && this.shortcuts.toggleSingleItem(event) && !item.selected,
!withinBoundingBox && item.selected && this.selectMode,
!withinBoundingBox && this.shortcuts.toggleSingleItem(event) && item.selected
];
// Usage
const shouldAdd = addRules.some(isMatch => isMatch)
// Usage inline
if (addRules.some(isMatch => isMatch)) {
// ...
} else if ...
Affected code
https://github.com/d3lm/ngx-drag-to-select/blob/c47dee4a49c5e0d300d4b54873e0347779736dda/projects/ngx-drag-to-select/src/lib/select-container.component.ts#L423-L441
Further thoughts.
The handling of withinBoundingBox
can be improved.
It is a repetitively used.
Maybe addRules
can be transformed to a list of functions that deal with withinBoundingBox
Hey @GregOnNet, thanks so much for opening this issue and proposing a refactoring for the messy conditional. I really appreciate it. I like the rule-based approach and I also agree that the withinBoundingBox
is quite repetitive. I also notice that some parts of the conditionals are sometimes just the inverse of parts of another rule.
Also, I need to give the conditionals better names, cause I find it hard after some time to understand what they do without spending a couple of minutes thinking about them 😂
Talking about names for rules brings classes to my mind. The taken class names below are incorrect, I believe. I don't know enough about the internals to come up with a better proposal.
interface BoxRule {
isMatch: (predicate: boolean) => boolean
}
class InsideBoxRule implements BoxRule {
isMatch(isInBox:boolean, predicate: boolean): boolean {
return isInBox && predicate;
}
}
class OutsideBoxRule implements BoxRule {
isMatch(isInBox:boolean, predicate: boolean): boolean {
return !isInBox && predicate;
}
Gotcha! Definitely an interesting approach. Thanks for your input, really appreciate it. I ll have to think about this for a while but I might refactor the library to use state machines. I think this is a great way to tame the complexity of this lib including all of its modes and features, as it's getting more and more feature rich and the conditionals are getting out of control.