Is Returning a Value from 'Next' a Bad Idea

is returning a value from 'next' a bad idea?

" Like the return and break keywords, next may be used alone, or
it may be followed by an expression or a comma-separated list of
expressions. When next is used in a loop, any values following
nextare ignored. In a block however the expression or expressions
become the "return value" of the yield statement that invoked the
block." (The Ruby Programming Language, David Flanagan & Yukihiro Matsumoto, 2008, page 150)

The book gives this example:

squareroots = data.collect do |x|
next 0 if x < 0 # return 0 for negative values
Math.sqrt(x)
end

and this alternative:

squareroots = data.collect do |x|
if (x < 0) then 0 else Math.sqrt(x) end
end

Standardized returning values - is it a good or bad idea

I'd say the premise of returning a boolean and something else is misguided.

A function should have a clear purpose with a clear result. If this result can be achieved, the result is returned. If the result cannot be achieved, the function either returns false or throws an exception. Which is better depends on the situation and your general error-handling philosophy. Either way, it's not typically useful to have a function return an error message. That message is not useful to the code that called the function.

PHP has its own mechanism to output error messages in addition to returning false results: trigger_error. It's purely a tool to aid debugging though, it doesn't replace the standard return value. It can be a good fit for cases where you want to display error messages purely to aid the developer though.

If a function is complex enough to possibly result in several different types of errors that need to be handled differently, you should use exceptions to do so.

For example, a very simple function with a clear purpose that only needs to return true or false:

function isUserLoggedIn() {
return $this->user == 'logged in';
}

A function with a purpose that may fail to fulfill that purpose:

function convertToFoo($bar) {
if (!is_int($bar)) {
return false;
}
// here be dragons
return $foo;
}

The same function that also triggers a message, useful for debugging:

function convertToFoo($bar) {
if (!is_int($bar)) {
trigger_error('$bar must be an int', E_USER_WARNING);
return false;
}
// here be dragons
return $foo;
}

A function that may legitimately run into several different kinds of errors that the calling code needs to know about:

function httpRequest($url) {
...

if (/* could not connect */) {
throw new CouldNotConnectException('Response code: ' . $code);
}

...

if (/* 404 */) {
throw new PageNotFoundException('Page not found for ' . $url);
}

return true;
}

And I'll paste this comment here as well:

It should not be the responsibility of the function to prepare, return
or display an end-user error message. If the purpose of the function
is to, say, fetch something from the database, then displaying error
messages is none of its business. The code that called the
fetch-from-database function merely needs to be informed of the
result; from here there needs to be code whose sole job it is to
display an error message in case the database function cannot get the
required information. Don't mix those two responsibilities.

Is returning 'None' for Python function ever a good idea?

This just flat-out isn't true. For one, any function that doesn't need to return a value will return None.

Beyond that, generally, keeping your output consistent makes things easier, but do what makes sense for the function. In some cases, returning None is logical.

If something goes wrong, yes, you should throw an exception as opposed to returning None.

Unfortunately, programming tends to be full of advice where things are over-generalized. It's easy to do, but Python is pretty good with this - practicality beats purity is part of the Zen of Python. It's essentially the use case for dict.get() - in general, it's better to throw the exception if a key isn't found, but in some specific cases, getting a default value back is more useful.

Is it necessarily bad style to ignore the return value of a method

It entirely depends upon what that return value is telling you and if that is important to know or not. If the data returned by the method is not relevant to the code that is calling it then ignoring it is entirely valid. But if it indicates some failure/counter/influential value then ignore it at your peril.

Is it OK not to handle returned value of a C# method? What is good practice in this example?

The returned value (or reference, if it's a reference type) is pushed onto the stack and then popped off again.

No biggy.

If the return value isn't relevant, you can safely do this.

But be sure that it isn't relevant, just in case.

Here's some code:

    static string GetSomething()
{
return "Hello";
}

static void Method1()
{
string result = GetSomething();
}

static void Method2()
{
GetSomething();
}

If we look at the IL:

Method1:

.locals init ([0] string result)
IL_0000: nop
IL_0001: call string ConsoleApplication3.Program::GetSomething()
IL_0006: stloc.0
IL_0007: ret

Method2:

IL_0000:  nop
IL_0001: call string ConsoleApplication3.Program::GetSomething()
IL_0006: pop
IL_0007: ret

Exactly the same number of instructions. In Method1, the value is stored in the local string result (stloc.0), which is deleted when it goes out of scope. In Method2, the pop operation simply removes it from the stack.

In your case of returning something 'really big', that data has already been created and the method returns a reference to it; not the data itself. In Method1(), the reference is assigned to the local variable and the garbage collector will tidy it up after the variable has gone out of scope (the end of the method in this case). In Method2(), the garbage collector can get to work, any time after the reference has been popped from the stack.

By ignoring the return value, if it really isn't needed, the garbage collector can potentially get to work sooner and release any memory that's been assigned. But there's very little in it (certainly in this case), but with a long running method, hanging onto that data could be an issue.

But far-and-away the most important thing is to be sure that the return value that you're ignoring isn't something that you should be acting on.

Is it bad practice to return from within a try catch finally block?

No, it's not a bad practice. Putting return where it makes sense improves readability and maintainability and makes your code simpler to understand. You shouldn't care as finally block will get executed if a return statement is encountered.

How to handle functions return value in Python

I have a number of problems with this function as written.

  1. It does two very different things: it either does some math and returns a useful answer, or it prints a message. That throws up some red flags already.

  2. It prints an error to stdout. Utility code should avoid printing if possible anyway, but it should never complain to stdout. That belongs to the application, not to spurious errors.

    If a utility function needs to complain and there's no other way to do it, use stderr as a last resort. (But in Python, you have exceptions and warnings, so there's definitely another way to do it.) Print-debugging is fine, of course — just make sure you delete it when you're done. :)

  3. If something goes wrong, the caller doesn't know what. It gets a None back, but that doesn't really explain the problem; the explanation goes to a human who can't do anything about it.

    It's also difficult to write code that uses this function correctly, since you might get a number or you might get None — and because you only get None when the input is bogus, and people tend not to think too much about failure cases, chances are you'll end up writing code that assumes a number comes back.

    Returning values of different types can be useful sometimes, but returning a value that can be a valid value or an error indicator is always a bad idea. It's harder to handle correctly, it's less likely that you will handle it correctly, and it's exactly the problem exceptions are meant to solve.

  4. There's no reason for this to be a function in the first place! The error-checking is duplicated effort already provided by float(), and multiplying by ten isn't such a common operation that it needs to be factored out. In fact, this makes the calling code longer.

So I would drop the function and just write this:

result = float('abc') * 10

Bonus: any Python programmer will recognize float, know that it might raise a ValueError, and know to add a try/except if necessary.

I know this was probably an artificial example from a book or homework or something, but this is why considering architecture with trivial examples doesn't really work — if you actually take it seriously, the whole example tends to disappear. :)

Not using a return value of a method , Is it bad design?

When you need to do something like this, chances are that the method does two things that are of value to a caller:

  1. Validates something, or produces another side effect, and
  2. Computes the result to be returned to the callers

Since some users need only #1, while other users need both #1 and #2, it may be a good idea to split the method in two parts, like this:

public void validatekUserGuess(String aGuess) {
// Some processing
}
public String checkUserGuess(String aGuess) {
validatekUserGuess(aGuess);
// Some additional processing
return result_of_a_guess;
}

Now the users that wish to ignore the return value would not be required to "pay" with CPU and memory for computing a value that they are going to discard anyway.

In C++, is it still bad practice to return a vector from a function?

Dave Abrahams has a pretty comprehensive analysis of the speed of passing/returning values.

Short answer, if you need to return a value then return a value. Don't use output references because the compiler does it anyway. Of course there are caveats, so you should read that article.

Is returning a value other than `self` in `__enter__` an anti-pattern?

TLDR: Returning something other than self from __enter__ is perfectly fine and not bad practice.

The introducing PEP 343 and Context Manager specification expressly list this as desired use cases.

An example of a context manager that returns a related object is the
one returned by decimal.localcontext(). These managers set the active
decimal context to a copy of the original decimal context and then
return the copy. This allows changes to be made to the current decimal
context in the body of the with statement without affecting code
outside the with statement.


The standard library has several examples of returning something other than self from __enter__. Notably, much of contextlib matches this pattern.

  • contextlib.contextmanager produces context managers which cannot return self, because there is no such thing.
  • contextlib.closing wraps a thing and returns it on __enter__.
  • contextlib.nullcontext returns a pre-defined constant
  • threading.Lock returns a boolean
  • decimal.localcontext returns a copy of its argument

The context manager protocol makes it clear what is the context manager, and who is responsible for cleanup. Most importantly, the return value of __enter__ is inconsequential for the protocol.

A rough paraphrasing of the protocol is this: When something runs cm.__enter__, it is responsible for running cm.__exit__. Notably, whatever code does that has access to cm (the context manager itself); the result of cm.__enter__ is not needed to call cm.__exit__.

In other words, a code that takes (and runs) a ContextManager must run it completely. Any other code does not have to care whether its value comes from a ContextManager or not.

# entering a context manager requires closing it…
def managing(cm: ContextManager):
value = cm.__enter__() # must clean up `cm` after this point
try:
yield from unmanaged(value)
except BaseException as exc:
if not cm.__exit__(type(exc), exc, exc.__traceback__):
raise
else:
cm.__exit__(None, None, None)

# …other code does not need to know where its values come from
def unmanaged(smth: Any):
yield smth

When context managers wrap others, the same rules apply: If the outer context manager calls the inner one's __enter__, it must call its __exit__ as well. If the outer context manager already has the entered inner context manager, it is not responsible for cleanup.


In some cases it is in fact bad practice to return self from __enter__. Returning self from __enter__ should only be done if self is fully initialised beforehand; if __enter__ runs any initialisation code, a separate object should be returned.

class BadContextManager:
"""
Anti Pattern: Context manager is in inconsistent state before ``__enter__``
"""
def __init__(self, path):
self.path = path
self._file = None # BAD: initialisation not complete

def read(self, n: int):
return self._file.read(n) # fails before the context is entered!

def __enter__(self) -> 'BadContextManager':
self._file = open(self.path)
return self # BAD: self was not valid before

def __exit__(self, exc_type, exc_val, tb):
self._file.close()

class GoodContext:
def __init__(self, path):
self.path = path
self._file = None # GOOD: Inconsistent state not visible/used

def __enter__(self) -> TextIO:
if self._file is not None:
raise RuntimeError(f'{self.__class__.__name__} is not re-entrant')
self._file = open(self.path)
return self._file # GOOD: value was not accessible before

def __exit__(self, exc_type, exc_val, tb):
self._file.close()

Notably, even though GoodContext returns a different object, it is still responsible to clean up. Another context manager wrapping GoodContext does not need to close the return value, it just has to call GoodContext.__exit__.



Related Topics



Leave a reply



Submit