cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Burwell <john.burw...@shapeblue.com>
Subject Re: [DISCUSS][PROPOSE] use of optional instead of null
Date Tue, 19 Jan 2016 03:44:53 GMT
Daan,

The problem lies within Java itself and the decision not to allow for typed nulls.  Therefore,
writing safe code requires a certain degree null checking.   There are also circumstances
where a null is the correct representation.  For example, there are optional numeric values
who can only be properly represented by null when they are not defined.  I think the following
would be a good start towards achieving the goal of not returning null:

        1. For strings, return blank (i.e. “”) for the missing case
        2. For collections, return an empty representation (e.g. Collections.emptySet(), Collections.emptyList(),
etc)
        3. Introduce a Nullable interface and create null implementation of classes we control
        4. For enumerations, provide a NONE value for the missing case

In my experience, one of the best ways to reduce null issues is to implement argument checking,
particularly on constructors using Guava’s Preconditions.checkArgument.  Not only does it
help ensure that inputs to classes fit the expected constraints, but it fails early — easing
the root cause identification.

Thanks,
-John

> On Jan 17, 2016, at 8:20 AM, Daan Hoogland <daan.hoogland@gmail.com> wrote:
>
> Thanks John, I agree that the solution from your gist is a good one in
> several circumstances as well. It doesn't completely invalidate both other
> solutions completely, though.
> so now we have three
> 1. exceptions instead of nulls
> 2. optionals
> 3. nullables
>
> Not sure how to continue from here, except that I think we need to remove
> all null returns where objects are expected.
>
>
> On Sat, Jan 16, 2016 at 1:56 PM, John Burwell <john.burwell@shapeblue.com>
> wrote:
>
>> Daan,
>>
>> I completely agree that returning null is bad. Not only does it yield a
>> ton of useless null checks, it creates leaky abstractions by spreading the
>> handling of the missing case out beyond the boundary of the class/subsystem.
>>
>> As a big proponent of the Null Object Pattern [1], I really wanted to like
>> Optional. It is a great concept in functional languages. So, I tried using
>> it in three (3) different projects since 2011. In all three systems, I
>> would say that, at best, it was no better than returning null, and, in
>> other cases, worse. Since Optional.get throws an exception when the wrapped
>> value is null, all optional accesses must be defensively checked so the
>> code base is littered with code like the following:
>>
>> if (value.isPresent()) {
>> return value.get();
>> }
>>
>> So, basically, you end up replacing null checks and NPEs with isPresent
>> checks and a Guava exception. As a bonus, when exceptions occurred in
>> production, we had explain the meaning of them. The quickest explanation —
>> “They are the new NPE”. For all new developers on the projects, we had one
>> more thing to explain to them which, again, was asking them to do something
>> differently with no added value. Based on these experiences, I prefer null
>> checks to optional.
>>
>> While it is more effort (i.e. more code), I have gone back to using the
>> Null Object Pattern implemented in this manner [2]. Not only does this
>> approach avoid NPEs, it also explicitly defines the behavior for the
>> missing case. For more complex examples, it can be unit tested to ensure
>> the missing case behaves as expected.
>>
>> Thanks,
>> -John
>>
>> [1]: https://en.wikipedia.org/wiki/Null_Object_pattern
>> [2]: https://gist.github.com/jburwell/f5162ad2d2de32c842b3
>>
>>>
>>
>> [image: ShapeBlue] <http://www.shapeblue.com>
>> John Burwell
>> ShapeBlue
>> d:  *+44 (20) 3603 0542 | s: +1 (571) 403-2411 *
>> <+44%20(20)%203603%200542%20%7C%20s:%20+1%20(571)%20403-2411>
>> e:  *john.burwell@shapeblue.com | t: *
>> <john.burwell@shapeblue.com%20%7C%20t:>  |  w:  *www.shapeblue.com*
>> <http://www.shapeblue.com>
>> a:  53 Chandos Place, Covent Garden London WC2N 4HS UK
>> Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue
>> Services India LLP is a company incorporated in India and is operated under
>> license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a
>> company incorporated in Brasil and is operated under license from Shape
>> Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of
>> South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is
>> a registered trademark.
>> This email and any attachments to it may be confidential and are intended
>> solely for the use of the individual to whom it is addressed. Any views or
>> opinions expressed are solely those of the author and do not necessarily
>> represent those of Shape Blue Ltd or related companies. If you are not the
>> intended recipient of this email, you must neither take any action based
>> upon its contents, nor copy or show it to anyone. Please contact the sender
>> if you believe you have received this email in error.
>>
>>
>> On Nov 17, 2015, at 11:16 AM, Daan Hoogland <daan.hoogland@gmail.com>
>> wrote:
>>>
>>> LS,
>>>
>>> As a spin off from a discussion in a PR where it is no longer relevant I
>>> made another PR to show the principle of the use of Optionals[1]
>>> <https://github.com/apache/cloudstack/pull/1060>
>>>
>>> Miguel from Schuberg Philis has been proposing this as replacement of the
>>> bad practice of returning null in methods and I agree. In seldom cases it
>>> might be more expedient to throw an exception, most notably when a null
>> is
>>> returned but no check against is done in the calling method. In those
>> cases
>>> throwing a CloudRuntimeException would be an easier way to go then the
>>> pattern in this PR. This is a runtime exception however so maybe creating
>>> an explicit one is more appropriate in those places.
>>>
>>> Anyway I want to propose to move to using the pattern from the PR from
>> now
>>> on.
>>>
>>> ​[1] https://github.com/apache/cloudstack/pull/1060​
>>>
>>> ​thoughts?​
>>> --
>>> Daan
>>
>> Find out more about ShapeBlue and our range of CloudStack related services:
>> IaaS Cloud Design & Build
>> <http://shapeblue.com/iaas-cloud-design-and-build//> | CSForge – rapid
>> IaaS deployment framework <http://shapeblue.com/csforge/>
>> CloudStack Consulting <http://shapeblue.com/cloudstack-consultancy/> | CloudStack
>> Software Engineering
>> <http://shapeblue.com/cloudstack-software-engineering/>
>> CloudStack Infrastructure Support
>> <http://shapeblue.com/cloudstack-infrastructure-support/> | CloudStack
>> Bootcamp Training Courses <http://shapeblue.com/cloudstack-training/>
>>
>
>
>
> --
> Daan

Find out more about ShapeBlue and our range of CloudStack related services:
IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//> |
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/> | CloudStack Software
Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
| CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>
Mime
View raw message