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

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@jjuran
Contributor
jjuran commented Feb 26, 2012

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

@jjuran jjuran Write Class->new as idiomatic Class::->new
Protect against cases like user code defining sub JSON, though primarily
this is a style patch.
bd98bae
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.

Contributor
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

Member

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 join this conversation on GitHub. Already have an account? Sign in to comment