I've been working on a project that makes relatively heavy use of nullable values. I've been using std.typecons.Nullable, and it mostly works well, but there are some improvements that could be made to the implementation:

* A toString() method (needed to fix bug #10915)
* An opEquals for comparisons with the type that the Nullable wraps
  * Currently, comparing a null Nullable!T with a T produces an error,
    but it makes more sense to just return false.
* Making isNull() @property

get() might also make more sense as a property, but not with its current name; it would be better to make the name a noun such as "value" rather than a verb. If it were to be changed, it could be done in a fully backward-compatible way by making "get" an alias of "value".

These would all be simple changes, so if someone's willing to guide me through the formalities, I could make this my first contribution to Phobos.

On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
> I've been working on a project that makes relatively heavy use of nullable values. I've been using std.typecons.Nullable, and it mostly works well, but there are some improvements that could be made to the implementation:
>
> * A toString() method (needed to fix bug #10915)
> * An opEquals for comparisons with the type that the Nullable wraps
>   * Currently, comparing a null Nullable!T with a T produces an error,
>     but it makes more sense to just return false.
> * Making isNull() @property
>
> get() might also make more sense as a property, but not with its current name; it would be better to make the name a noun such as "value" rather than a verb. If it were to be changed, it could be done in a fully backward-compatible way by making "get" an alias of "value".
>
> These would all be simple changes, so if someone's willing to guide me through the formalities, I could make this my first contribution to Phobos.

The wiki has a pretty good guide of the overall process: http://wiki.dlang.org/Pull_Requests

On Tuesday, 8 October 2013 at 19:20:05 UTC, Brad Anderson wrote:
>
> The wiki has a pretty good guide of the overall process: http://wiki.dlang.org/Pull_Requests

That answers most of my questions, but it seems a little... informal. I guess the formal review process doesn't really apply to minor API changes.
If it's really that easy, I'll just go for it.

On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
> I've been working on a project that makes relatively heavy use of nullable values. I've been using std.typecons.Nullable, and it mostly works well, but there are some improvements that could be made to the implementation:
>
> * A toString() method (needed to fix bug #10915)
> * An opEquals for comparisons with the type that the Nullable wraps
>   * Currently, comparing a null Nullable!T with a T produces an error,
>     but it makes more sense to just return false.
> * Making isNull() @property
>
> get() might also make more sense as a property, but not with its current name; it would be better to make the name a noun such as "value" rather than a verb. If it were to be changed, it could be done in a fully backward-compatible way by making "get" an alias of "value".
>
> These would all be simple changes, so if someone's willing to guide me through the formalities, I could make this my first contribution to Phobos.

Or we could just nuke the alias this. A Nullable!T isn't a T. It's a T handler. "alias this" allows implicit cast, which should only happen with a "is a" relation. Using it in a different context (such as nullable) is wrong, and these errors are the price we are paying for it. It's a bit more verbose, but it would solve *all* of these ambiguity and unexpected error problems.

I would much rather we push in that direction instead.

This also holds true for RefCounted btw.

That's my take on it.

On Tuesday, 8 October 2013 at 20:55:35 UTC, monarch_dodra wrote:
>
> Or we could just nuke the alias this. A Nullable!T isn't a T. It's a T handler. "alias this" allows implicit cast, which should only happen with a "is a" relation. Using it in a different context (such as nullable) is wrong, and these errors are the price we are paying for it. It's a bit more verbose, but it would solve *all* of these ambiguity and unexpected error problems.

Maybe...

An object reference can be null, but a null "Object" isn't really an Object but the absence of one. With "alias this", the behavior meshes with the behavior of object references. Whether that's good or bad, it does add some consistency.

I'm not sure what I'd like to see in the long term, but I'll leave the "alias this" in for my pull request because removing it would be a breaking change.

On Tue, 08 Oct 2013 22:55:34 +0200
"monarch_dodra" <monarchdodra@gmail.com> wrote:
> 
> A Nullable!T isn't a T. It's a T handler.

I see that as an (unavoidable) implementation detail.

> "alias this" allows implicit cast, which should only happen with a "is a" relation. Using it in a different context (such as nullable) is wrong, and these errors are the price we are paying for it. It's a bit more verbose, but it would solve *all* of these ambiguity and unexpected error problems.
> 
> I would much rather we push in that direction instead.
> 
> This also holds true for RefCounted btw.
> 
> That's my take on it.

Personally, I find Nullable's "alias this" functionality to be a wonderful convenience. FWIW.

On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
> * Making isNull() @property

Hmm... looks like it's already @property. I guess this happened after the last update to the Phobos docs.

I'll still need to fix the other stuff, though.

On Wednesday, 9 October 2013 at 03:42:50 UTC, Nick Sabalausky wrote:
> Personally, I find Nullable's "alias this" functionality to be a
> wonderful convenience. FWIW.

Yeah, it's convenient to be able to switch out T with Nullable(T) and have it work without breaking the API... Well, it sort of works, when the stars align and the blessing of Donald Knuth is upon you.

On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
> I've been working on a project that makes relatively heavy use of nullable values. I've been using std.typecons.Nullable, and it mostly works well, but there are some improvements that could be made to the implementation:
>
> * A toString() method (needed to fix bug #10915)
> * An opEquals for comparisons with the type that the Nullable wraps
>   * Currently, comparing a null Nullable!T with a T produces an error,
>     but it makes more sense to just return false.

OK, so that's two functions already. What about opCmp? What about toHash?

What if T is a range? Then "Nullable!T.empty" should return true if the Nullable is empty. IF we don't, we'll get a crash in foreach.

> On Tue, 08 Oct 2013 22:55:34 +0200
> "monarch_dodra" <monarchdodra@gmail.com> wrote:
> > 
> > A Nullable!T isn't a T. It's a T handler.
> 
> I see that as an (unavoidable) implementation detail.

Is it though? C++ has done without it, and is still doing without it. It has "implicit build from" which every one says is mostly an abomination. Then here we are, bashing on their implicit constructors, yet using "implicit cast to" O_o.

> Personally, I find Nullable's "alias this" functionality to be a
wonderful convenience. FWIW.

I draw the line when convenience gets in the way of my programs not crashing.

On Wednesday, 9 October 2013 at 06:48:31 UTC, monarch_dodra wrote:
> On Tuesday, 8 October 2013 at 19:04:33 UTC, BLM768 wrote:
>> I've been working on a project that makes relatively heavy use of nullable values. I've been using std.typecons.Nullable, and it mostly works well, but there are some improvements that could be made to the implementation:
>>
>> * A toString() method (needed to fix bug #10915)
>> * An opEquals for comparisons with the type that the Nullable wraps
>>  * Currently, comparing a null Nullable!T with a T produces an error,
>>    but it makes more sense to just return false.
>
> OK, so that's two functions already. What about opCmp? What about toHash?
>
> What if T is a range? Then "Nullable!T.empty" should return true if the Nullable is empty. IF we don't, we'll get a crash in foreach.
>
>> On Tue, 08 Oct 2013 22:55:34 +0200
>> "monarch_dodra" <monarchdodra@gmail.com> wrote:
>> > 
>> > A Nullable!T isn't a T. It's a T handler.
>> 
>> I see that as an (unavoidable) implementation detail.
>
> Is it though? C++ has done without it, and is still doing without it. It has "implicit build from" which every one says is mostly an abomination. Then here we are, bashing on their implicit constructors, yet using "implicit cast to" O_o.
>
>> Personally, I find Nullable's "alias this" functionality to be a
> wonderful convenience. FWIW.
>
> I draw the line when convenience gets in the way of my programs not crashing.

I think that there are some situation where the aliased nullable is just very handy, especially when you are adapting previous written code, but all that considerations are interesting.

It would be wonderful to have some sort of linked "rationale", with pro and versus, with suggested use cases and possible pitfall, just in the ddoc section of the module: some sort of community-driven wiki page?
--
Paolo Invernizzi