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

X::MyModule::Foo or MyModule::X::Foo ? #57

Open
AlexDaniel opened this issue Jul 2, 2019 · 19 comments
Open

X::MyModule::Foo or MyModule::X::Foo ? #57

AlexDaniel opened this issue Jul 2, 2019 · 19 comments
Labels
fallback If no other label fits

Comments

@AlexDaniel
Copy link
Member

This question comes up relatively often (IRC discussion):

Which package should my module's exceptions go into? X::MyModule or MyModule::X? […] Is there a best practice?

(very) rough estimates:

Even considering all false matches, seems like MyModule::X style is not very popular.

Should there be a recommendation somewhere in the docs to use X::MyModule style? Or are there any good arguments for MyModule::X?

@AlexDaniel AlexDaniel added the fallback If no other label fits label Jul 2, 2019
@AlexDaniel AlexDaniel self-assigned this Jul 2, 2019
@japhb
Copy link

japhb commented Jul 2, 2019

FWIW, I prefer to treat X:: as a "parallel namespace" next to the regular class namespace, because X:: in the lead position is visually distinctive, almost sigil-like; it calls out that Exceptional Stuff is Happening Here. When it's buried in the middle it gets lost -- especially since a great many modules have at least two level names at the module's top level, plus at least one level of submodules below it, so with ::X:: you'd really end up with ModuleCategory::MyModule::MySubModule::X::ExceptionName or ModuleCategory::MyModule::X::MySubModule::ExceptionName, both of which totally lose the visual distinctiveness of being an exception.

@AlexDaniel AlexDaniel added documentation Big changes related to official Raku documentation and removed fallback If no other label fits labels Jul 13, 2019
@AlexDaniel AlexDaniel assigned JJ and unassigned AlexDaniel Jul 13, 2019
@AlexDaniel
Copy link
Member Author

AlexDaniel commented Jul 13, 2019

@JJ sounds like something that can be done in the docs. Please leave this ticket here in case we want to come back to this discussion. But yeah, if X::MyModule:: is a more common form, and people tend to prefer it more, and there are no technical reasons for it to be differently (it seems), then let's just recommend it.

@ugexe, any thoughts?

@ugexe
Copy link
Contributor

ugexe commented Jul 13, 2019

From what I remember there used to be (still is?) bugs with using X::

@AlexDaniel
Copy link
Member Author

Yeah, I vaguely remember something like that too, but what kind of bugs? Can't find anything now.

@taboege
Copy link
Member

taboege commented Jul 13, 2019

I remember this from not long ago, but this seems kinda special and avoidable with proper namespacing maybe?

@AlexDaniel
Copy link
Member Author

Yes, but in that case it was X::IO, while here the discussion is about the difference between X::MyModule::IO and MyModule::X::IO.

@ugexe
Copy link
Contributor

ugexe commented Jul 14, 2019

ping @jonathanstowe -- does any of this ring a bell? Maybe jonathanstowe/Tinky@8bfc778 ?

@AlexDaniel
Copy link
Member Author

I think it's about this ticket. The commit resolved the name clash between two modules, and the namespace indeed made a difference (“Note that if it's a name other than X then it's OK”).

@jonathanstowe
Copy link

yep, I put the Raku/doc#2865 to discuss how to improve the documentation on this. It's the things like :

class Foo {
    class X::Common is Exception {
     }
}

That cause the problem. I've converged on making it X::Foo::Common to avoid the risk of collision.

@AlexDaniel
Copy link
Member Author

Ah! I just stumbled upon a dup: Raku/user-experience#11

@Xliff
Copy link

Xliff commented Jul 17, 2019

While I do agree that it is nice to have X:: Because it is visually distinctive, it can be confusing. From the previous issue, how does one resolve X::User::Access::Auth::Invalid? Really, it should be re-written to X::User::Access:AuthInvalid which would solve all problems, but we can't mandate that to users who want to add namespaces to their own exceptions.

User::Access::X::* makes things less confusing. You still have the value of ::X:: (although it is not as visible), but then Excpetion namespaces in User::Access::X:: are now distinctive.

That's my own 2¢, though. Thanks for listening.

@vrurg
Copy link
Contributor

vrurg commented Jul 17, 2019

@Xliff I think the point here is X::Module notation is recommended but not constrained.

BTW, it is arguable which notation is more confusing. I'm inattentive and might easily overlook the ::X:: inside while X:: is much clearer to me. Besides, The X in the middle might mean other things. Think of Words::A::A .. Words::Z::Z module family, for instance or anything else like that. Whereas X:: is reserved and some automation tools may rely on this to do their job.

@bbkr
Copy link

bbkr commented Jul 18, 2019

@vrurg This Words::X example is extremely rare edge case, while namespace conflicts is quite common.

From my experience - I have Module::X::WhatHappened as exception naming standard in my $dayjob, both in huuuge Perl 5 and small Perl 6 codebase. And this method is superior in every aspect:

  • not accidental collisions
  • not opaque (as in X::User::Access::Auth::Invalid example)
  • easy to inherit
  • easy to move around if namespace has changed (no need to transform two directory trees)
  • easy to remove all related exceptions if namespace is no longer needed

@vrurg
Copy link
Contributor

vrurg commented Jul 18, 2019

@bbkr may I remind you that nothing is obligatory here?

* not accidental collisions

X::Module::. Though I agree: it reduces the risk but doesn't make it 0.

* easy to inherit

Don't see what makes it easier?

* easy to move around if namespace has changed (no need to transform two directory trees)

This is not a point whatsoever. Sometimes renaming a namespace needs so many concomitant changes that one more directory to rename is not an argument. ;)

* easy to remove all related exceptions if namespace is no longer needed

Same thing. You have the namespace of X::Module:: and it is easiliy distinguishable.

My own conlusion: this is all is mostly about one's taste. And as any discussion about different taste it leads to nowhere. The primary points of this thread are: X::Module:: is (or must be) recommended but not enforced; it could simplify certain automation tasks by making exception names clearly distinguishable to a script.

@ugexe
Copy link
Contributor

ugexe commented Jul 18, 2019

it could simplify certain automation tasks by making exception names clearly distinguishable to a script.

but that is not the case for anything else -- doing it in this instance would not be consistent with namespaces not being owned or magical (and is still useless to tooling for indirect names). And using the X:: namespace has caused issues in the past, so its not just a matter of taste. I personally agree with @bbkr, and indeed follow those same patterns in both Perl 5 and 6 real life code bases.

@bbkr
Copy link

bbkr commented Jul 18, 2019

@bbkr may I remind you that nothing is obligatory here?

Yes. I'm aware that we're discussing best practices.

Don't see what makes it easier? [Inheritance]

Let's assume you have trading platform, where users can trade items and services.

To prevent all actions forbidden by law you made exceptions: X::User::LegalAgeTooLow (when underaged user tries to buy alcohol), X::Item::LegalOwnershipNotConfirmed (when user tries to sell house without notarial entry), X::Item::LegalTheftReport (user tries to sell stolen item), etc. Mixed with a bunch of common exceptions, such as X::Item::DescriptionTooShort.

As you add more an more exceptions you realise that it's good to have Legal exceptions separated to have them in one place and keep up with law changes. So you made X::User::Legal and exceptions that inherit from it: X::User::Legal::AgeTooLow, X::User::Legal::ComplaintFilled, etc.

Then you want to do the same with items, so X::Item::Legal is created. But your items are organized. You have Item::Car, Item::Beer, Item::Legal (for example law counselling service). And that leads to ugly namespace mashup if you want to add specific legal exceptions for specific items, such as X::Item::Beer::Legal::AfterExpiryDate and... X::Item::Legal::Legal::NoLicense.

After reorganizing into Module::X::WhatHappened model it makes more sense, because you have Item::Beer::X::Legal::AfterExpiryDate and Item::Legal::X::Legal::NoLicense.

What I'm trying to show is that you can add inheritance levels on both left and right side of ::X:: without even thinking about naming collisions: Item::Legal::Counselling::X::Legal::NoLicense, Item::Legal::Counselling::X::Legal::License::Expired, etc.

@jnthn
Copy link
Contributor

jnthn commented Jul 18, 2019

easy to move around if namespace has changed (no need to transform two directory trees)

There's no need for two directory trees in either form. I've been using the X:: as a prefix form, and typically then write them in the same module as the code that will the throwing them.

@JJ
Copy link
Contributor

JJ commented Aug 20, 2019

  1. I don't think this is a documentation issue per se. In the documentation we can advise people to use one or the other, but people will eventually use "Exception::For::Class" or "Exception4class" or whatever.
  2. It might be a language issue, because there are ~250 X::Y::Z exceptions in Rakudo. If a change is decided, we would have to start there first.
  3. I'm very much for following the current convention.
    Also, I'm de-labelling and de-self-assigning it. Please feel free to re-assign it to me if you really this is doc's turf.

@JJ JJ removed their assignment Aug 20, 2019
@JJ JJ removed the documentation Big changes related to official Raku documentation label Aug 20, 2019
@AlexDaniel AlexDaniel self-assigned this Aug 20, 2019
@AlexDaniel AlexDaniel added the fallback If no other label fits label Aug 20, 2019
@jonathanstowe
Copy link

Yeah,if anything I'd document it as a "Trap" and leave it on the grounds of no consensus.

@AlexDaniel AlexDaniel assigned jnthn and unassigned AlexDaniel May 14, 2020
@lizmat lizmat unassigned jnthn Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fallback If no other label fits
Projects
None yet
Development

No branches or pull requests

10 participants