Skip to content
This repository has been archived by the owner on Jan 24, 2018. It is now read-only.

remove unnecessary interface #54

Closed
wants to merge 1 commit into from
Closed

remove unnecessary interface #54

wants to merge 1 commit into from

Conversation

browner12
Copy link
Contributor

the interface doesn’t do anything, and i can’t find it referenced in
any of the tests. removing the interface and references to it.

the interface doesn’t do anything, and i can’t find it referenced in
any of the tests. removing the interface and references to it.
@sebastianbergmann
Copy link
Owner

This is a so-called marker interface.

@browner12
Copy link
Contributor Author

i saw that in the comments and was doing some research on them, since I hadn't heard of them previously. while it seems debated whether they are a good practice or not, I could not find actual use of the marker throughout the code. is there a spot it is actually being used?

@browner12
Copy link
Contributor Author

yah, saw those. still just don't really understand the point. from my (very) brief intro into marker interfaces, my understanding is that it allows you to check that some class implements the marker interface. so something like:

if($someClass instanceof Exception)

but I don't see that anywhere. would it be better to simply use the built in php exception that all exceptions are extending from?

if($someClass instanceof \Exception)

if you do need to check for that?

@sebastianbergmann
Copy link
Owner

In this case the Money's components exception classes are marked, using the marker interface, as "belonging to" or "coming from" the Money component. This can be useful for type declarations used in catch statements.

@browner12
Copy link
Contributor Author

hmm.. okay. that's interesting and I can see the use there. thanks for clearing that up for me. the only other thing I'd say about it then is I think it would be much clearer if you make the name more explicit. For example, call the interface MoneyException. this will help avoid confusion and naming collision, because I'm sure the base \Exception is used quite often.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants