[Webkit-unassigned] [Bug 188501] Meaning of OptionSet::contains is unclear when used with OptionSet argument

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188501

--- Comment #5 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
(In reply to Antti Koivisto from comment #4)
> > I think contains() should act like containsAll(). When I say
> 
> You may think that, but the fact the it didn't is a pretty strong indication
> that not everyone agrees. Better use clear names.
> 

The verb contains means: "have or hold (someone or something) within". Your implementation make OptionSet::contains() means "contains some of".

> > I do not think this is needed. The bitwise & operator overloading can do
> > exactly what this function does. When I say
> > 
> >     if (option1 & options2)
> 
> contains*() functions are useful because they return bools. For example 
> 
> bool foo = option1 & options2; 

How often such expression is used in our code base? I rarely see such use.

And I do not think it is good design to have two operations does the same thing.

if (option1 & options2)
if (option1.contsinas(options2))


In fact the second one will be semantically wrong if option1 is a sub set of option2 because your implementation for contains() makes it commutative operation although it should not. For example if option1 = 0x1 and option2 = 0xff

if (option1.contsinas(options2))

will be true even semantically it should be false.  In this case 

if (option1 & options2)

will be make more sense and the order of arguments will not matter.

> 
> doesn't compile, you need a clunky explicit conversion:
> 
> bool foo = !!(option1 & options2);

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180813/3b72979d/attachment.html>