Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jan 22, 2013

http://d.puremagic.com/issues/show_bug.cgi?id=6224

Nothing too controversial. But since the returned type is a struct I figure it's better to throw an exception rather than the user getting a segfault when they try to call ownerTid.send().

Additionally I've removed Tid.send, it was duplicated code but isn't required anymore since UFCS can take its place.

/**
* Thrown when $(D ownerTid) doesn't find an owner thread.
*/
class OwnerTidMissing : Exception
Copy link
Contributor

Choose a reason for hiding this comment

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

By convention, we append Error or Exception to exception classes.

Copy link
Author

Choose a reason for hiding this comment

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

Well that convention seems to be only partially followed, e.g. MessageMismatch, OwnerTerminated, LinkTerminated, PriorityMessageException, MailboxFull.

But why are we using some form of Hungarian notation in a statically typed language?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is common convention for many, many languages, regardless of static or dynamic typing. It's helpful because you don't have to go and look at the class definition to figure out what a PriorityMessage might be. The Exception lets you know immediately. It also avoids conflating exception types with the general type 'namespace'.

I don't know why half of the types in this module don't follow the convention.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing that is very helpful (to me), is knowing whether the object thrown is an Exception or an Error. The fact that it is in the name keeps things self documented.

@ghost
Copy link
Author

ghost commented Jan 23, 2013

I've renamed it to TidMissingException. This will make it usable for any future functionality (e.g. if we ever add the ability to iterate child threads but one of them exits right before we check its Tid).

@monarchdodra
Copy link
Collaborator

Whilst we're adding exceptions, how about a NullTidError ? This would be thrown when you call send with an empty Tid. It sure as hell beats having an object violation.

Just throwing it out there.

@@ -303,6 +303,19 @@ class MailboxFull : Exception
}


/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if maybe this should be an Error? Isn't it a logical error to try to get a Tid when none is present?

Copy link
Author

Choose a reason for hiding this comment

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

I can't say for sure, but I think a thread which spawned another one could exit while the child thread is still running. In that case ownerTid would likely be empty. I'll cc Sean about it: @complexmath

Copy link
Contributor

Choose a reason for hiding this comment

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

@complexmath ping?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should not add to Errors without good reason. Let's keep this an exception.

alexrp added a commit that referenced this pull request Mar 8, 2013
Issue 6224 - Add ownerTid() property.
@alexrp alexrp merged commit 9cb2065 into dlang:master Mar 8, 2013
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.

4 participants