|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/5] xen/common: introduce a new framework for save/restore of 'domain' context
On 01.04.2020 14:16, Julien Grall wrote:
> Hi Jan,
>
> On 01/04/2020 13:07, Jan Beulich wrote:
>> On 01.04.2020 14:00, Julien Grall wrote:
>>> On 27/03/2020 18:50, Paul Durrant wrote:
>>>> + if ( (exact ?
>>>> + (dst_len != c->desc.length) : (dst_len < c->desc.length)) ||
>>>
>>> Using ternary in if is really confusing. How about:
>>>
>>> dst_len < c->desc.length || (exact && dst_len != c->desc.length) ||
>>>
>>> I understand that there would be two check for the exact case but I think
>>> it is better than a ternary.
>>
>> I'm of the opposite opinion, and hence with Paul. While the alternative
>> you suggest is still reasonable because of the special case here, I
>> find it confusing / more difficult to read / follow
>>
>> if ( (a && b) || (!a && c) )
>>
>> (and I've seen quite a few instances of such over time) instead of
>>
>> if ( a ? b : c )
>
> If the ternary was the only condition and in a single line then it would be
> okay. However, the if is split over 3 lines...
>
> The more stuff you put in an if, then more chance you are going to
> misread/make a mistake (you likely know what I am referring about here ;)).
>
> So if you prefer the ternary, then we should at least write 2 ifs.
Since it's || that would be fine (albeit not preferred) by me. If
it was &&, would be be suggesting two nested if()-s (which
generally in reviews we ask to be avoided)? I see nothing wrong
with e.g.
if ( (a ? b : c) &&
(x ? y : z) )
Nevertheless I agree that very large conditionals often are more
difficult to read.
Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |