Skip to content
This repository has been archived by the owner on Dec 22, 2021. It is now read-only.

Write Class->new as idiomatic Class::->new #28

Closed
wants to merge 1 commit into from
Closed

Write Class->new as idiomatic Class::->new #28

wants to merge 1 commit into from

Conversation

jjuran
Copy link
Contributor

@jjuran jjuran commented Feb 26, 2012

Protect against cases like user code defining sub JSON, though primarily
this is a style patch.

Protect against cases like user code defining sub JSON, though primarily
this is a style patch.
@kchodorow
Copy link
Contributor

Perl doesn't seem to like that, I get:

Bareword "MongoDB::Database::" refers to nonexistent package at ~/perl/blib/lib/MongoDB/Connection.pm line 546.

running make test.

@jjuran
Copy link
Contributor Author

jjuran commented Feb 27, 2012

On Feb 27, 2012, at 12:24 PM, Kristina wrote:

Perl doesn't seem to like that, I get:

Bareword "MongoDB::Database::" refers to nonexistent package at ~/
perl/blib/lib/MongoDB/Connection.pm line 546.

The BUILD subroutine contains what amounts to eval "use MongoDB::Database";. I'm haven't used Moose, so there may be some
explanation I'm simply unaware of, but I'm wondering if it wouldn't
be simpler to just use MongoDB::Database at the top of
Connection.pm. I suppose it comes down to the question of whether
it's an advantage to defer loading MongoDB::{Database,OID,Timestamp}
until a Connection is created. (MongoDB::Cursor is already imported
at compile-time, so its appearance in BUILD() does nothing.)

Josh

@edaniels
Copy link
Contributor

This isn't necessarily idiomatic to perl but more of a mistake that could happen. Our current codebase only uses a capital letter in the first character of an identifier for class names/package names and constants. That combined with testing has protected us from any issues where perl would interpret the bareword as a subroutine. Similarly, user perl code that is tested shouldn't run into this issue as well.

That said I'm going to close this PR but if there is a use case that absolutely makes using this a better move, please provide one :)

@edaniels edaniels closed this Jul 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants