NSelene icon indicating copy to clipboard operation
NSelene copied to clipboard

Condition negation aka Not(Condition) implementation details and design

Open yashaka opened this issue 4 years ago • 1 comments

in #49 we started to add "negating conditions" support...

The first straightforward implementation was:

namespace NSelene.Conditions
{
    public class Not<TEntity> : DescribedCondition<TEntity>
    {
        private readonly DescribedCondition<TEntity> condition;

        public Not(DescribedCondition<TEntity> condition)
        {
            this.condition = condition;
        }

        public override bool Apply(TEntity entity)
        {
            return ! condition.Apply(entity);
        }

        public override string DescribeActual()
        {
            return $"not {condition.DescribeActual()}";
        }

        public override string DescribeExpected()
        {
            return $"not {condition.DescribeActual()}";
        }
    }
}

This is pretty straightforward implementation... under nselene hood, its usage is hidden under Be.Not.* and Have.No.*, so if we write something like:

S("#foo").Should(Be.Not.Enabled);

everything looks awesome

but if somebody created their own condition named for example Match.Higthlighted, and don't want to predefine Match.Not.Highlighted, but just use:

S("#foo").Should(new Not<SeleneElement>(Match.Highlighted));

... now it's starting to look awkward, bulky, over-verbose, because of:

  • requirement of specifying generic type argument (<SeleneElement>)
  • mandatory new

if we create somewhere the Not as static generic method, then it would become much more readable:

S("#foo").Should(Not(Match.Highlighted));

The question is – where to put it?

Something like ...

S("#foo").Should(Condition.Not(Match.Highlighted));

... would be nice, but will fail to compile, requiring to change to:

S("#foo").Should(Condition<SeleneElement>.Not(Match.Highlighted));

... also not allowing to use static import for more concise:

S("#foo").Should(Not(Match.Highlighted));

putting it to something like:

class ConditionUtils 
{
    // does not look sexy:)
}

yet, this is probably the only implementation option if we want to allow for users the .Should(Not(Match.Highlighted)) style... Moreover, if adding Not as static method, then for lesser confusion it would be better to rename class Not to something like class Negated (any other naming ideas? maybe class Inverted...)

Yet not sure to do all this or not...

But look what we also can do:

S("#foo").Should(Match.Highlighted.Not);

How is it? :)

Here Not is a property returning => new Not<SeleneElement>(this);

Dead concise:) But is it readable enough?

Pretty not a "sexy English style", but:

  • powerful
  • all conditions just being inherited e.g. from DescribedCondition should receive this .Not as built-in
  • pretty concise, probably the most concise without additional implementation (like Be.Not.*)

Probably in most cases we can create aliases like Be.Not.*, and an nselene user also can create similar aliases on his side... So this "awkwardness" should not be a big issue...

Look also at this example to feel how it might taste in the future with all bullets on board:

S("#foo").Should(Be.Blank.And(Have.Text("")).And(Disabled.Not));

good enough?

yashaka avatar Jun 29 '20 16:06 yashaka

So far the original Not condition class is made internal. I have also added Condition#Not property allowing to write something like S("#foo").Should(Be.Visible.Not), yet keeping it internal...

Guys, let's finalize the naming of the Not condition class and Condition#Not property so we finally can make them public and release...

yashaka avatar Jul 06 '20 21:07 yashaka