-1

I have the following initialisation code in a system tray applet:

use Gtk3 -init;
use Glib::Object::Introspection;

eval {
    Glib::Object::Introspection->setup(
        basename => 'Notify',
        version => '0.7',
        package => "MyProgram::Notify",
        );
};

if ($@) {
    say "no notify because setup failed: $@";
    $use_notify = 0;
} else {
    MyProgram::Notify->init();
}

That code is based upon fdpowermon, but seems to come more or less from the Exception handling examples in the POD of Glib::Object::Introspection.

But perlcritic (at level 3) argues about it:

Return value of eval not tested at line …

So I tried to rewrite that with Try::Tiny:

use Gtk3 -init;
use Glib::Object::Introspection;
use Try::Tiny;

try {
    Glib::Object::Introspection->setup(
        basename => 'Notify',
        version => '0.7',
        package => "MyProgram::Notify",
        );
} catch {
    say "no notify because setup failed: $@";
    $use_notify = 0;
} finally {
    if (!$@) {
        MyProgram::Notify->init();
    }
}

But then perl argues:

Can't locate object method "new" via package MyProgram::Notify::Notification

While I do see that especially the finally block is not a real improvement, I do not understand why using Try::Tiny makes such a difference with regards to the package created by Glib::Object::Introspection.

Or is there a better way than Try::Tiny to make this code more elegant and more readable while keeping perlcritic happy?

brian d foy
  • 129,424
  • 31
  • 207
  • 592
Axel Beckert
  • 6,814
  • 1
  • 22
  • 23
  • @ikegami: Yes, the call to MyProgram::Notify::Notification->new seems to be part of the Glib::Object::Introspection magic. Thanks for the example code, but it seems to be even less readable to me. :-/ – Axel Beckert Jan 28 '17 at 02:15
  • "_But perlcritic (at level 3) argues about it: Return value of eval not tested at line …_" -- why would one have to test the return value? That's not the prescribed way to check for exception, nor the only proper one. The critic should've also checked whether there is any code between `eval`'s closing `};` and `if ($@)` before complaining. Do you have to go by perlcritic, and by the "harsh" level? – zdim Jan 28 '17 at 03:56

2 Answers2

3

The whole point of the critique is to avoid checking $@ because it could have been clobbered. Yet after all your changes, you're still checking $@!

Worse, Try::Tiny puts the error in $_, not in $@, and only in catch blocks.

I think what's happening is that MyProgram::Notify->init() is being called when it shouldn't because of the above bugs.

Fix:

my $use_notify = 1;
try {
    Glib::Object::Introspection->setup(
        basename => 'Notify',
        version => '0.7',
        package => "MyProgram::Notify",
    );

    MyProgram::Notify->init();
} catch {
    say "no notify because setup failed: $_";
    $use_notify = 0;
}

or

my $use_notify = 1;
try {
    Glib::Object::Introspection->setup(
        basename => 'Notify',
        version => '0.7',
        package => "MyProgram::Notify",
    );
} catch {
    say "no notify because setup failed: $_";
    $use_notify = 0;
}

MyProgram::Notify->init() if $use_notify;

Without Try::Tiny:

my $use_notify = 1;
if (!eval {
    Glib::Object::Introspection->setup(
        basename => 'Notify',
        version => '0.7',
        package => "MyProgram::Notify",
    );

    MyProgram::Notify->init();

    1;  # No exception
}) {
    say "no notify because setup failed: " . ( $@ // "Unknown error" );
    $use_notify = 0;
}

or

my $use_notify = 1;
if (!eval {
    Glib::Object::Introspection->setup(
        basename => 'Notify',
        version => '0.7',
        package => "MyProgram::Notify",
    );
    1;  # No exception
}) {
    say "no notify because setup failed: " . ( $@ // "Unknown error" );
    $use_notify = 0;
}

MyProgram::Notify->init() if $use_notify;
ikegami
  • 367,544
  • 15
  • 269
  • 518
  • Using `$@` in the `finally` block comes from [Try::Tiny's POD](https://metacpan.org/pod/Try::Tiny#EXPORTS). – Axel Beckert Jan 28 '17 at 02:23
  • I already tried the first variant before, too, and it failed for me. Your second example indeed looks like the right solution. I do set `$use_notify`, but only use it later despite I could have used already immediately. – Axel Beckert Jan 28 '17 at 02:24
  • 1
    Re "*Using `$@` in the `finally` block comes from Try::Tiny's POD*", Read your on link again. It doesn't say anything remotely similar to that. It actually says "*`$@` does **not** contain the error.*", "*Inside the `catch` block the caught error is stored in `$_`*", and "*note that the `finally` block does not localize `$_` with the error*". It also explicitly mentions that `$@` is not changed at all – ikegami Jan 28 '17 at 02:26
  • Let's not get stuck on the `$@`. I really like your first two examples, but neither works for me and also aborts with the `Can't locate object method "new" via package "MyProgram::Notify::Notification"` error, too. So my question what's actually causing the difference still stands. I guess I first need to understand where that object comes from. Just checked with printf-debugging: `MyProgram::Notify->init();` in the original code is successfully called, i.e. works there. – Axel Beckert Jan 28 '17 at 02:41
0

Actually the answer to my question is basically the same as this one: A missing semicolon behind the catch (or rather finally) block.

Sorry for the noise.

Community
  • 1
  • 1
Axel Beckert
  • 6,814
  • 1
  • 22
  • 23
  • 2
    That would have been easier to pinpoint had you included more code. Because the closing curly looked like the last line of the program it shouldn't have mattered. – simbabque Jan 28 '17 at 07:11