All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Sayers <andrew-git@pileofstuff.org>
To: Subho Banerjee <subs.zero@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Tim Henigan <tim.henigan@gmail.com>, git <git@vger.kernel.org>
Subject: Re: Git.pm
Date: Thu, 10 May 2012 21:55:10 +0100	[thread overview]
Message-ID: <4FAC2B2E.7060101@pileofstuff.org> (raw)
In-Reply-To: <CAB3zAY3VHtUobJfJ7=nSKb_6uJOXLGVHzR18qV6txPkzf54cDw@mail.gmail.com>

Try::Tiny is an increasingly standard part of Perl - for example, it's
used extensively in Moose.  There's a good list of arguments about why
you should use it instead of eval in the Try::Tiny documentation:
http://search.cpan.org/~nuffin/Try-Tiny-0.01/lib/Try/Tiny.pm

Now I've got that talking point done, here's what I really think :)

Try::Tiny is designed on the assumption that throwing and catching
objects is something people should do all the time, and it can cause
subtle errors that are only worth the hassle if you get a lot of benefit
from doing so.  It's easy enough to come up with ideas for where they
might be useful, but in the real world advanced uses for exceptions are
usually a sign you're doing it wrong.  Three of the most common reasons
for frequent/complex exceptions are handling errors further up the call
stack, recovering from operations that fail, and clever error-handling.


If you want exceptions to be caught by code further up the call stack
than the immediate caller, you're likely to be disappointed.  This is
one of the places where "separation of concerns" applies - if I use a
module that uses a module that uses your module, then catching
exceptions from your code will just cause my program to break when some
module in the middle obscures your error by adding its own layer of
error handling.


If you have an operation that really might fail, and you want to
encourage most people to handle it most of the time, it's better to have
a function with a meaningful name and good documentation.  This puts the
burden on the calling function to handle the error instead of letting
them think "oh well, if it dies someone else will handle it".  It also
forces you to split functions along boundaries that make your code
readable, instead of falling for the temptation to make something that
"just works"... until it doesn't, and the maintainer has to go
spelunking through code they don't know.  So instead of:

   try {
       Foo::frobnicate( widgets => 3 );
   } catch {
      if (ref($_) eq 'Error::Widget') {
          die "Could not add 3 widgets";
      }
   }

It's better to ask the people using your module to write:

   my $foo = Foo->new;
   $foo->add_widgets(3) or die "could not add 3 widgets"
   $foo->frobnicate;

This is easier to document, easier to write and easier to read.


If you have an operation where calling code is supposed to do something
more complicated than give up, it's better to use a callback.  This
gives you an opportunity to document what's needed, and to check that
the calling code is doing the right thing before it's too late.  So
instead of:

    my $widgets = 3;
    while ( $widgets ) {

        try {
            Foo::frobnicate($widgets);
            $widgets = 0;
        } catch {
            if ( $_->{remaining_widgets} < 2 ) {
                die $_->{error};
            } elsif ( $_->{remaining_widgets} == 2 ) {
                $widgets = 0;
            }
        }

    }

It's better to ask people using your module to write:

    Foo::Frobnicate(
        widgets => 3,
        error_handler => sub {
             my ( $remaining_widgets, $error ) = @_;
             die $error if $broken_widgets < 2;
             return "give up" if $remaining_widgets == 2;
             return "continue";
        },
    );

Again, this is more readable and easier to document.


Aside from the philosophical angle, Try::Tiny is particularly hard to
maintain because it looks like a language extension, but is actually
just an ordinary module.  The try {} and catch {} blocks are anonymous
subroutines, which lead to some wonderfully unintuitive behaviour.  See
what you think these do, then run the code to find out:


    sub foo {
        try {
            return 1;
        }
        return 0;
    }

    sub bar {

        our @args = @_;
        our @ret;

        try {
            @ret =
                wantarray
                    ?   grep( /blah/, @args )
                    : [ grep( /blah/, @args ) ]
        };

        return @ret;
    }

    my $foo = "bar";
    sub baz {
        my $foo = "baz";
        try {
            print $foo;
        }
    }

    sub qux {
        my $ret;
        try {
            $ret = "value";
        }
        return $ret;
    }

    print foo, "\n";
    print bar( "blah", "blip" ), "\n";
    baz;
    print qux, "\n";


In short, Try::Tiny looks like a lot of gain for not much pain, but
actually it's the other way around.

	- Andrew

  parent reply	other threads:[~2012-05-10 20:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-26  4:15 Git.pm Subho Banerjee
2012-04-26 18:41 ` Git.pm Randal L. Schwartz
2012-04-26 18:58 ` Git.pm Tim Henigan
2012-04-26 20:10   ` Git.pm Subho Banerjee
2012-04-26 20:31     ` Git.pm Jonathan Nieder
2012-05-10 13:19       ` Git.pm Subho Banerjee
2012-05-10 15:16         ` Git.pm Jonathan Nieder
2012-05-10 15:54         ` Git.pm demerphq
2012-05-10 16:18           ` Git.pm Subho Banerjee
2012-05-10 17:22             ` Git.pm demerphq
2012-05-10 16:20           ` Git.pm Junio C Hamano
2012-05-10 17:38             ` Git.pm demerphq
2012-05-10 20:55         ` Andrew Sayers [this message]
2012-05-11  8:27           ` Git.pm demerphq
2012-05-11 16:56         ` Git.pm Randal L. Schwartz
2012-05-11 18:10           ` Git.pm Junio C Hamano
2012-05-19  7:08             ` [PATCH][GIT.PM 1/3] Ignore files produced from exuberant-ctags Subho Sankar Banerjee
2012-05-19  7:08               ` [PATCH][GIT.PM 2/3] Getting rid of throwing Error::Simple objects in favour of simple Perl scalars which can be caught in eval{} blocks Subho Sankar Banerjee
2012-05-19  9:38                 ` Andrew Sayers
2012-05-23 11:02                   ` Subho Banerjee
2012-05-23 19:36                     ` Andrew Sayers
2012-05-19  7:08               ` [PATCH][GIT.PM 3/3] Perl code uses eval{}/die instead of Error::Simple and Git::Error::Command Subho Sankar Banerjee
2012-04-26 19:17 ` Git.pm Junio C Hamano
2012-04-26 19:59 ` Git.pm Sam Vilain
  -- strict thread matches above, loose matches on Subject: below --
2008-11-19 17:56 Git.pm nadim khemir
2008-11-20  8:34 ` Git.pm Petr Baudis
2008-11-20 13:07   ` Git.pm Petr Baudis
2008-11-23 19:58   ` Git.pm nadim khemir
2008-12-07 17:39     ` Git.pm nadim khemir
2008-11-21  2:56 ` Git.pm Jakub Narebski
2008-11-23 20:09   ` Git.pm nadim khemir

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FAC2B2E.7060101@pileofstuff.org \
    --to=andrew-git@pileofstuff.org \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=subs.zero@gmail.com \
    --cc=tim.henigan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.