kentnl / File-ShareDir-ProjectDistDir

Simple set-and-forget using of a '/share' directory in your projects root
Other
5 stars 5 forks source link

Return a Path::Class::* object #4

Open xenoterracide opened 11 years ago

xenoterracide commented 11 years ago

I thought I mentioned this once, maybe on RT, the request was to be able to configure ::ProjectDistDir to be able to return Path::Class objects so that I don't have to create one from the path returned afterwards. Did this ever become possible? if not consider this a request to make it happen.

kentfredric commented 11 years ago

You did, but it must have been in email, because I can't find it anywhere now.

Was added in 0.4.0, but still poorly documented.

 use File::ShareDir::ProjectDistDir ':all', defaults => {   pathclass => 1 };

and

 use File::ShareDir::ProjectDistDir  dist_dir => { pathclass => 1 };

should both work

kentfredric commented 11 years ago

I think in a release somewhere after 0.5 I might remove Path::Class returns from the basic implementation, because they're needlessly complicating the implementation details.

Trying to do too much via the exporter interface proves to be very messy, and messier still if you're writing code that needs to inject functionality into other modules.

As it is, I'm breaking out large chunks of implementation detail into an Object Oriented class, so people who want advanced behaviour can use that instead of the import() interface, and people who have simpler requirements can continue to use the import() interface.

As an example how exporter can be messy, take for instance this, what I had before I restructured Path::IsDev to have an object:

https://metacpan.org/source/KENTNL/Path-FindDev-0.1.0/lib/Path/FindDev.pm#L55

sub _build_find_dev {
  my ( $class, $name, $arg ) = @_;

  require Path::IsDev;
  my $isdev = do {
    my $args = {};
    $args->{set} = $arg->{set} if $arg->{set};
    ## no critic (ProtectPrivateSubs)
    Path::IsDev->_build_is_dev( 'is_dev', $args );
  };

  return _build_find_dev_all( $class, $name, { %{$arg}, isdev => $isdev } );

}

I'm doing some pretty ugly tricks and breaking encapsulation here just to make it work.

Now, compare that with the version after rehashing Path::IsDev

https://metacpan.org/source/KENTNL/Path-FindDev-0.2.0/lib/Path/FindDev.pm#L17

sub _build_find_dev {
  my ( $class, $name, $arg ) = @_;

  my $object;
  return sub {
    my ($path) = @_;
    $object ||= do {
      require Path::FindDev::Object;
      Path::FindDev::Object->new($arg);
    };
    return $object->find_dev($path);
    }
}

The latter is much tidier imo.

And restructuring Path::FindDev into an object as well has let me implement a bunch of features that would be a nightmare to implement with a pure exporter

https://metacpan.org/source/KENTNL/Path-FindDev-0.2.0/lib/Path/FindDev/Object.pm

xenoterracide commented 11 years ago

I haven't seriously started using it yet, so that's fine, I just always thought it seemed silly to wrap the statement in Path::Class after.