Skip to content

Commit

Permalink
Make IO::Path.dir throw rather than fail
Browse files Browse the repository at this point in the history 
In general, if something is returing an Iterable, we throw rather than fail
to prevent exactly the problem that R#2650 describes.  Is spectest clean, so
we weren't too strict about this anyway.
  • Loading branch information
@lizmat
lizmat committed Jan 27, 2019
1 parent 71a19c6 commit 38f4b7b
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions src/core/IO/Path.pm6
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,7 @@ my class IO::Path is Cool does IO {
proto method dir(|) {*} # make it possible to augment with multies from modulespace
multi method dir(IO::Path:D: Mu :$test = $*SPEC.curupdir) {
CATCH { default {
fail X::IO::Dir.new(
:path($.absolute), :os-error(.Str) );
X::IO::Dir.new(:path($.absolute), :os-error(.Str)).throw
} }

my str $dir-sep = $!SPEC.dir-sep;
Expand Down

5 comments on commit 38f4b7b

@salortiz
Copy link
Contributor

@salortiz salortiz commented on 38f4b7b Mar 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hop isn't to late to be against.
I used to wrote

with '/some/dir'.IO.dir -> @content {
...
} else { 
   .fail
}

In general I think that every throw prevents the handling with with reducing its usefulness.

@AlexDaniel
Copy link
Contributor

@AlexDaniel AlexDaniel commented on 38f4b7b Mar 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@salortiz interesting… can you file a ticket, please? At the very least we probably want to grep through the ecosystem for lines like that.

In general I think that every throw prevents the handling with with reducing its usefulness.

But I don't think it prevents much. Just add a try (like with try '/some/dir'.IO.dir …)

@salortiz
Copy link
Contributor

@salortiz salortiz commented on 38f4b7b Mar 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexDaniel, using with try hides the Failure in the else block, please compare:

with try +"a" { ... } else { say $_ ~~ Failure } # False, $_ is Nil

with

 with  +"a" { ... } else { say $_ ~~ Failure }  # True

While a Failure can be handled cleanly with with, //, orelse, etc. or opted-out by the fatal pragma or, in this case, a simple type constraint; throw should be handled with try and $! or with a CATCH block, both polluting the lexical scope.

Try to rewrite, with the current status the elegant:

with 'optional'.IO.dir // 'existent'.IO.dir -> @content { … }

It is a matter of flexibility and choices to the user. In my opinion, Perl6's Exception/Failure model is an important differentiator, despite not being very widespread nor well documented.

A simple my Str @foo = ... would have sufficed to the original #2650 reporter.

I'm thinking in fill a ticket in spec to encourage the discussion in search of 'best-practices' in this matter.

Any thoughts @lizmat?

@Leont
Copy link
Contributor

@Leont Leont commented on 38f4b7b Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this wouldn't suffice?

CATCH { .fail }

@salortiz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a CATCH block modifies the semantic of the lexical scope around it, I use only when I needs centralized handling for, potentially many unrelated, Exceptions.

This is all about different paradigmatic positions.

My main stance in this 'thrown Exception' vs 'return Failure' is that the former, as a one-way road, forces the user to use the CATCH paradigm, the latter allows others. TMTOWTDI

Please sign in to comment.