Best practices for warnings about wrong context

vsespb has asked for the wisdom of the Perl Monks concerning the following question:

Let's suppose I've created a library with the following API:

sub myfunc
{
  my ($ref) = @_;
  # modify $ref somehow
  warn "Should be called in void context" if defined wantarray;
}
[download]

The function does something, but does not returns anything useful. It returns nothing. I can add "return ;" but this does not change things - function returns nothing and I don't want it to be used in non-void context. Purpose of the last warning in function is simple: the function returns nothing, and if someone called it in scalar/list context, he's doing something wrong. So this warning adds more strictness to user's code:

examples of user's code:

example 1:

func1();
myfunc($data);
func2();
[download]

everything ok - no warning

example 2:

func1(myfunc($data));
[download]

there will be the warning! user obvious did something wrong here!

example 3:

sub anotherfunc {
  # .. code here
  myfunc($data)
}

func1(anotherfunc())
[download]

there will be the warning! and indeed this code isn't sane.

So far everything going well, and the warning seems useful.

However, let's look on Catalyst now:

sub xxx : Local Args(0) {
    my ($self, $c, $s, $r, $p) = @_;

    $c->res->body( wantarray );
}
[download]

It's the Catalyst action, which return value is ignored, but it's called in list context.

Sometimes action return value is important (see "Any data returned from the action forwarded to, will be returned by the call to forward." in manual), sometimes no. But catalyst always call actions in list context.

Now the quesions:
(a) Is that good practice to create library, which warns for "defined wantarray" for functions, returning nothing?
(b) Are libraries like catalyst obligated to never call user's callback in non-void context if it's value is ignored?
I assume if (b)=no, then (a)=no.
Is there a convention for such kind of things? (or let me rephrase: if I am getting this warning when using my library with Catalyst, who's fault is that? the library's or Catalyst's)

Comment on Best practices for warnings about wrong context Select or Download Code

Replies are listed 'Best First'.
Re: Best practices for warnings about wrong context (work-around) by tye (Sage) on Oct 05, 2015 at 01:24 UTC
Seems a reasonable warning to me. It is also reasonable to worry that there can be cases where the user doesn't control the context. So you might want to use the more advanced features of warnings (warnings::register) to allow users to disable that warning. For call-backs, you can also easily avoid the warning via the simple trick of: `specifyCallback( sub { myfunct(...); return } );` [download] - tye	[reply] [d/l]
Re: Best practices for warnings about wrong context by dsheroh (Monsignor) on Oct 05, 2015 at 08:44 UTC
I can't imagine how (b) could be a "yes". Different callbacks are naturally used in different contexts. Sometimes it makes sense for a callback to return a value, other times it doesn't. And sometimes (like Catalyst), the framework is generic enough that the framework designer can't know in advance whether a return value is expected from a given callback or not. All that aside, I lean towards "no" on (a) regardless. The entire concept of issuing a warning for calling a sub in the wrong context rubs me the wrong way on a rather deep level. I can't identify exactly why, but it's probably connected to the fact that "doing something that doesn't make sense to the module's author" does not necessarily imply "doing something wrong".	[reply]
Re^2: Best practices for warnings about wrong context by mr_mischief (Monsignor) on Oct 05, 2015 at 16:31 UTC
Maybe I've taken references to a few different subs and passed them around, and then at some point call whichever one has been passed? Maybe some other sub returns a scalar, a hashref, a list, or whatever. What happens if I want to use the sub that refuses to be used in list context or scalar context? Well I can wrap it in another sub, maybe an anonymous sub, which doesn't seem like a heart-rending, mind-warping workaround. Should wrapping it be necessary? Who's to say, really? If someone wants to return any kind of user data or status information, it would require the wrapper anyway since the author didn't return anything from the original sub. One might as well have the wrapper call it in void context then return whatever you like.	[reply]
Re: Best practices for warnings about wrong context by ikegami (Patriarch) on Oct 06, 2015 at 02:43 UTC
Note that the warnings is easily silenced in situations such as the Catalyst example you gave by following up the call with an explicit `return;`. `sub xxx : Local Args(0) { my ($self, $c) = @_; ... myfunc(); return; }` [download]	[reply] [d/l] [select]
Re^2: Best practices for warnings about wrong context by vsespb (Chaplain) on Oct 06, 2015 at 08:25 UTC
yes,sure	[reply]
Re: Best practices for warnings about wrong context by Your Mother (Archbishop) on Oct 05, 2015 at 12:38 UTC
This Catalyst code is not sensible. `sub xxx : Local Args(0) { my ($self, $c, $s, $r, $p) = @_; $c->res->body( wantarray ); }` [download] The arg handling is wrong `Args(0)` means just that, and there is no way using the return of `wantarray` could ever make sense as a body. If that is meant to be a stand-in for the void context function you speak about it should not be there but by itself. `sub xxx : Local Args(0) { my ($self, $c) = @_; function_in_void_context(); # And should be wrapped in model probably. # Naked function calls in Cat is code smell. $c->res->body("Value or none if falling back to template, etc"); }` [download]	[reply] [d/l] [select]
Re^2: Best practices for warnings about wrong context by vsespb (Chaplain) on Oct 05, 2015 at 16:39 UTC
there is no way using the return of wantarray could ever make sense as a body Of course it makes sense. It prints "1" as body, means wantarray returns TRUE. This function is for PoC purpose.	[reply]
Re^3: Best practices for warnings about wrong context by Your Mother (Archbishop) on Oct 05, 2015 at 16:50 UTC
It does not. The body expects a string/scalar. `$c->response->body(defined wantarray);` prints "1". Maybe I am missing the point because a setter like `->body` expects a value and is therefore never going to be a void context call; it's a strange question / concept to want to demonstrate.	[reply] [d/l] [select]
Re^4: Best practices for warnings about wrong context by vsespb (Chaplain) on Oct 05, 2015 at 16:55 UTC
Re^5: Best practices for warnings about wrong context by Your Mother (Archbishop) on Oct 05, 2015 at 17:23 UTC
Some notes below your chosen depth have not been shown here
Re: Best practices for warnings about wrong context by Anonymous Monk on Oct 05, 2015 at 12:39 UTC
If the program cannot rightly be used in a certain context, I suggest that it should `die()` so that a traceback to the offending statement can be made ... and the presence of the problem identified. A warning message is never going to be seen in a server log, and, if it were, it would not be useful.	[reply]