Writing functions (and by 'function' we also include 'method') can be fraught with problems: bad definition, misuse of parameters, bad return values, unused return values, misnaming, name collisions, etc. We strongly suggest that you follow our guidelines when defining and using functions. Violating these guidelines may still produce valid Python code, but not everything that is allowed in Python is necessarily a good idea to implement in practice.
Changing the value of a parameter in a function before using it misses the entire point of the function. A parameter provides either data to process or a location to store data. Overwriting a parameter destroys whatever is being sent into the function. In the following code:
def array_to_stack(stack, source, n):
"""
-------------------------------------------------------
Pushes contents of source onto stack.
-------------------------------------------------------
Parameters:
stack - a Stack object (Stack)
source - a Python list (list)
n - maximum number of values to copy (int)
Returns:
None
-------------------------------------------------------
"""
source = [] # destroys source parameter
stack = Stack() # destroys stack parameter
n = int(input("Enter n: ")) # destroys n parameter
...
the source
, stack
, and n
parameters are all being destroyed by being overwritten before they
are even used. If a function has a parameter, it is there to be used.
Overwriting parameters indicates a fundamental lack of understanding
of how functions work. Using input on a function that does not require
input indicates a fundamental lack of understanding of what this
function does. Note that an IDE normally flags this with an error or
warning. Eclipse underlines these parameters and provides the warning
Unused parameter.
Unlike the previous issue, the parameter value is used, but it is changed later in the code. Changing the value of a parameter in a function does not change that parameter's value outside of the function. The following code:
def change(n):
n = n * 2 # changes value of n parameter, but only while function is executing
return
value = 5
print(f"value: {value}")
print("change(value)")
change(value)
print(f"value: {value}")
prints:
value: 5 change(value) value: 5
This is just how Python works. It is important, then, to not expect a parameter value to be changed by a function.
In a sense, then, it is 'safe' to change a parameter's value inside of a function since that new value will not be preserved outside of the function. However, this is poor programming practice. Parameters should have their values and meanings preserved. The following example does neither:
def read_data(line):
"""
-------------------------------------------------------
Parameters:
line - a comma-delimited string containing valid field values (str)
-------------------------------------------------------
"""
line = line.strip().split(",") # changes string parameter 'line' to a list
title = line[0]
# ...
The parameter line
is changed from a string into a list,
which means it is both poorly named and no longer true to its
docstring description. Using a separate variable for the split line
makes code more readable and consistent:
def read_data(line):
"""
-------------------------------------------------------
Parameters:
line - a comma-delimited string containing valid field values (str)
-------------------------------------------------------
"""
data = line.strip().split(",") # creates new list variable, 'line' unchanged in value and type
title = data[0]
# ...
Changing the contents of a parameter is not the same as changing the value of a parameter. You may change the contents of a Python list or data structure, and the changes to the contents are preserved. Thus:
def array_to_stack(stack, source):
while len(source) > 0:
v = source.pop() # changes contents of source
stack.push(v) # changes contents of stack
return
is fine, but:
def bad_func(stack, source):
...
source = [] # changes value of source - does not survive function
stack = Stack() # changes value of stack - does not survive function
...
return
is not, as the comments indicate. Changing a parameter value in this way usually means you are either destroying the parameter or misusing the parameter, as noted in the previous points.
Good function documentation as used in courses CP104/CP164/CP213 tells you:
Do what the documentation tells you. Pay attention in particular to the types. Here is a an example of a typical error:
def split(self):
"""
-------------------------------------------------------
Returns:
target1 - a new List with >= 50% of the original List (List)
target2 - a new List with <= 50% of the original List (List)
-------------------------------------------------------
"""
target1 = [] # wrong type of list
target2 = List() # correct type of List
...
The docstring for this method specifically notes that the return
values of target1
and target2
are to be List
s,
i.e. objects as defined by the List
class. Instead the
code assigns an empty Python list to target1
.
List()
≠ []
List()
≠ list()
Similarly, pay close attention to the parameter types and make sure the values the function processes are the types of values required. Here is an example of a typical error:
def get_by_year(movies, year):
"""
-------------------------------------------------------
Parameters:
movies - a list of Movie objects (list of Movie)
year - the Movie year to select (int)
-------------------------------------------------------
"""
for movie in movies:
data = movie.split("|")
...
The Parameters description explicitly states that the parameter movies
must be a (list of Movie)
,
i.e. a Python list of Movie
objects. However, the line
data = movie.split("|")
calls the split
method on what should be a Movie
object. The Movie
class definition does not contain a split
method. This
code is clearly assuming that the data in movies
is a
list of strings of the form:
["Dark City|1998|Alex Proyas|7.8|0", "Zulu|1964|Cy Endfield|7.8|9", ...]
A string is not a Movie
object. The following is a Movie
object:
movie = Movie("Dark City", 1998, "Alex Proyas", 7.8, [0])
Somewhere, somehow, the string data must be converted to the required object type by calling the appropriate class constructor. Don't mistake data for objects. Unprocessed data is not an object.
Mistaking a list of strings (["1","5","9"]
) as a list of
integers ([1,5,9]
) - or visa-versa - is another problem
for both parameters and return values.
Printing is the not the same as Returning. If the function doesn't
specifically require values to be printed, don't call print()
.
Input from the user is not the same as having parameters passed into a
function. If the function doesn't specifically require values to be
read from user input, don't call input()
.
Not using a return value is always a problem - how do you know if a function works if you don't look at its return value - but is particularly an issue in recursive functions. The following function won't return the correct count if its return value is not used as part of its general case:
def _count_aux(self, node):
"""
---------------------------------------------------------
Returns the number of nodes in a BST.
-------------------------------------------------------
Parameters:
node - a BST node (_BST_Node)
Returns:
count - number of nodes (int)
----------------------------------------------------------
"""
if node is None:
count = 0
else:
count = 1
self._count_aux(node._left) # return value not used
self._count_aux(node._right) # return value not used
return count
The correct code that makes use of the returned values is:
count = 1 + self._count_aux(node._left) + self._count_aux(node._right)
The opposite error is to use a return value when there isn't one, as in this example:
some_list = some_list.sort()
The built-in Python sort
method sorts this contents of
its list (in this example, some_list
) in place. The
contents of the list are sorted when the method is finished. The
method actually returns None
, so assigning the result of
the method to some_list
destroys the contents of the
list. The fix is to know what the method actually returns and using it
correctly.
A function name should never be reused as a variable name. The following is bad practice:
def diameter(radius):
diameter = radius * 2
return diameter
diameter = diameter(5)
print(diameter)
diameter = diameter(6)
print(diameter)
In this example the first call to the function diameter
returns the value 10
. The second call throws the error:
diameter = diameter(6) ^^^^^^^^^^^ TypeError: 'int' object is not callable
The line:
diameter = diameter(5)
changes
the definition of diameter
from a function to an int
,
thus the error message. Strictly speaking, the reuse of diameter
within the function definition doesn't cause a problem, but in general
a coding behaviour that causes problems under certain circumstances is
best avoided altogether, particular if the rules related to its
'correct' use are complex or obscure. Like reusing a parameter noted
above, the reuse of an identifier that changes its definition from a
function to a non-function is simply bad programming practice.
Methods must be indented properly within a class definition. All
methods that belong to a class must be indented within the class
definition line. Methods defined with the wrong indentation will not
be executed as they will not be seen as part of the class, as in this
example:
class Structure:
def __init__(self):
...
def compare(self, target):
...
s = Structure()
t = Structure()
print(s.is_empty())
print(s.compare(t))
generates the error:
print(s.compare(t)) ^^^^^^^^^ AttributeError: 'Structure' object has no attribute 'compare'
Because the compare
function is not indented within the
class definition, it is not considered to be part of the class
definition, thus the error message. (Python simply treats self
as another variable and thus does not flag its use outside of a class
definition as an error.)
There may be function requirements not documented within the docstring, either because of the context the function is being written in, general standards that are to be met, or some other situation. Functions must be written to meet these requirements. A typical requirement might be:
You may not use or refer to the internal Stack elements such as _values
.
Thus a function that contains code like:
if
source._values:
violates these requirements and may suffer a
significant grade penalty, including a 0 grade. Requirements are very
important!