proposal: refactor conditionals

[GregOnNet]

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

ngx-drag-to-select/projects/ngx-drag-to-select/src/lib/select-container.component.ts

Lines 423 to 441 in c47dee4

	const shouldAdd =
	(withinBoundingBox &&
	!this.shortcuts.toggleSingleItem(event) &&
	!this.selectMode &&
	!this.selectWithShortcut) ||
	(withinBoundingBox && this.shortcuts.toggleSingleItem(event) && !item.selected) ||
	(!withinBoundingBox && this.shortcuts.toggleSingleItem(event) && item.selected) ||
	(withinBoundingBox && !item.selected && this.selectMode) ||
	(!withinBoundingBox && item.selected && this.selectMode);
	const shouldRemove =
	(!withinBoundingBox &&
	!this.shortcuts.toggleSingleItem(event) &&
	!this.selectMode &&
	!this.selectWithShortcut) ||
	(!withinBoundingBox && this.shortcuts.toggleSingleItem(event) && !item.selected) ||
	(withinBoundingBox && this.shortcuts.toggleSingleItem(event) && item.selected) ||
	(!withinBoundingBox && !item.selected && this.selectMode) ||
	(withinBoundingBox && item.selected && this.selectMode);

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

[d3lm]

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 😂

[GregOnNet]

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

[d3lm]

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.