Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a simple NotNull struct #477

Closed
wants to merge 10 commits into from
Closed

add a simple NotNull struct #477

wants to merge 10 commits into from

Conversation

adamdruppe
Copy link
Contributor

I've talked about implementing these a lot of times on the newsgroup, but I don't see one in Phobos yet!

This is my latest idea on how to implement a NotNull type with tests and documentation, added
to std.typecons (right below Nullable).

I haven't actually used any NotNull in practice, but from the attached tests, it looks like it will work well - hence, the pull request.

I'm open to any comments, however.

try
{
takesNotNull(notNull(test)); // test is null now, so this should throw an assert failure
assert(0, "it let us pass a null value to a NotNull function");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going to be caught by the catch clause as well? See assertThrown and friends.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right! Duh. Changed to assertThrown!AssertError

@ghost
Copy link

ghost commented Mar 5, 2012

Shouldn't assert(rhs !is null); be changed to enforce(rhs !is null);?

@adamdruppe
Copy link
Contributor Author

I was thinking of the not null as being similar to a contract; it looks for usage errors in the program rather than something like user input, so assert is the thing. I think assert is right.

@alexrp
Copy link
Member

alexrp commented Mar 18, 2012

FWIW, I agree that we should use assert for this.

}
}

/// A convenience function to construct a NotNull value. If you pass it null, it will throw.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "it will assert"?

@dnadlinger
Copy link
Member

Maybe an assumeNonNull/checkNonNull pair would provide a nice unified entry point to the NonNull universe?

@JakobOvrum
Copy link
Member

How about adding an opAssign overload for T (which would assert)?

Also, this type does not work for all possible types T, thus it should have a template constraint.

@adamdruppe
Copy link
Contributor Author

I actually had the opAssign before, but removed it in tonight's commit. The reason is you might want the type system to catch an accidental assignment so you can make a decision - check null or assume null - at that time and write it explicitly.

The same logic could apply to the constructor, but the fact that you can't use auto is annoying enough as it is, and having to do NotNull!T t = assumeNotNull(new T()); is probably just too far.

@JakobOvrum
Copy link
Member

I agree, the transition from nullable to non-nullable should be explicit when possible, it just seemed a little out of sync with the constructor. I suppose construction isn't a problem because the name of the type, NotNull, will always be explicit at least once.

}

/// A convenience function to construct a NotNull value from something you know isn't null.
NotNull!T assumeNotNull(T)(T t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function should contain an assert(t !is null)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor asserts.

// have used a regular int*, but with the assurance that
// it never stored null.
---
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are tabs in the comments. Please replace them with spaces.

@andralex
Copy link
Member

andralex commented Jul 2, 2012

This seems to be languishing. @adamdruppe, still there?

@adamdruppe
Copy link
Contributor Author

I've been very busy the last couple months with a lot of
real life stuff.

Going to be settling down again soon; moving this week
and then thing should start returning to normal.

On Sun, Jul 01, 2012 at 07:02:27PM -0700, Andrei Alexandrescu wrote:

This seems to be languishing. @adamdruppe, still there?


Reply to this email directly or view it on GitHub:
#477 (comment)

@andralex
Copy link
Member

andralex commented Jul 9, 2012

waiting

@andralex
Copy link
Member

Will close this for now, please reopen as fit. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants