Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Next revision
Previous revision
lfa:2023:coding-style [2023/11/03 12:47]
mihai.udubasa [Import, variable, member or method redefinitions and shadow-ing]
lfa:2023:coding-style [2023/11/06 16:07] (current)
mihai.udubasa [Example code]
Line 8: Line 8:
  
 This category encapsulates code which is very vulnerable to errors, even if, in the case of your submission, the code behaves as expected. This category encapsulates code which is very vulnerable to errors, even if, in the case of your submission, the code behaves as expected.
 +
 <note important>​ <note important>​
 The presence of too may of these will lead to your submission being penalised with up to 0.1 points per stage. The presence of too may of these will lead to your submission being penalised with up to 0.1 points per stage.
Line 15: Line 16:
 The following are the types of errors which WILL be flagged from this category: The following are the types of errors which WILL be flagged from this category:
  
-==== Using a variable before its definition ​====+==== General errors ​====
  
-This covers all cases of accessing a variable before a value is assigned ​to it for the first time. Due to the lack of a compile-time type checksuch errors ​may slip in undetected in the code. +These are bits of code which will lead to unexpected behaviour or crashes, but, due to the interpreted nature ​of the python language, may not be detected when running ​the code normally (as that piece of code may not be executed). 
-<hidden > + 
-<code python ​example.py>+This includes: 
 +<​hidden ​Use of variables or class members before they are defined
 +<code python>
 x = a + 1   # a is not yet defined x = a + 1   # a is not yet defined
  
Line 31: Line 34:
     def inc(self):     def inc(self):
         self.m += self.n ​ # A does not have a member named '​n'​.         self.m += self.n ​ # A does not have a member named '​n'​.
 +    def init_x(self):​ 
 +        self.x = 5  # any member variables should be defined in the init method 
 +    def use_x(self):​ 
 +        self.x += 1  # this will cause an error if init_x is not called beforehand
 </​code>​ </​code>​
 </​hidden>​ </​hidden>​
  
-==== Importvariable, member or method ​redefinitions ​and shadow-ing ====+<hidden import errors>​ 
 +This includes trying to import inexistent modules or importing objects which do not exist in the given module. Another issue which, depending on the circumstance,​ may cause problems, is cyclic imports (two or more modules import eachother in a cyclic manner, meaning that a given module ends up depending on itself). The first type of errors may go undetected if imports are placed inside functions, classes or control blocks. To avoid this, make sure all of your imports are placed at the beggining of your file, outside of any control blocks.  
 +</​hidden>​ 
 + 
 +<hidden function and method refefinitions>​ 
 +It is an error to define a function with the same name multiple times in a single module (exception being inner functions, i.e. functions defined inside other functions) and, likewise, to define multiple methods with the same name inside the same class (overriding an inherited method is exempt from this). Another possible error may be defining a member of a class with the same name as one of its methods. This will make the method uncallable (any reference to that name will point to the member instead of the method) 
 +</​hidden>​ 
 + 
 +==== Import ​and variable redefinitions ​(shadow-ing==== 
 + 
 +This covers cases where an already-defined object is redefined in an erronous way. If the creation of a new object is the desired behaviour, then consider renaming it to avoid conflicts:​ 
 +Defining a variable in an inner scope (inside a function or method) will cause any variable (including imports) with the same name to be temporarly unaccessible. This usually is done by accident, but if this is the intended behaviour, you should consider renaming the variable to avoid confusion.
  
-This covers cases where an already-defined object is redefined in an erronous way: 
 <hidden import and variable shadowing and redefinitions>​ <hidden import and variable shadowing and redefinitions>​
 <code python> <code python>
  
-import re as regex +import re
 matches = set() matches = set()
  
-def is_digits(input_string):​ +def split_digits(input_string):​ 
-    ​regex = '​(0|1|2|3|4|5|6|7|8|9)*' ​ # the 'regex' variable defined here '​shadows'​ or hides the name of the module re +    ​re = '​(0|1|2|3|4|5|6|7|8|9)*' ​ # the 're' variable defined here '​shadows'​ or hides the name of the module re. any references to the name '​re'​ within this function will refer to this string 
-    ​match regex.fullmatch(regex, input_string) # this should thow an errorbut if the '​regex'​ variable happened to thave the fullmatch methodthis could slip in undetected  +    ​split_str ​re.split(regex, input_string) # this calls the string split methodrather than the split function of the re modulewhich in this case is not the intended behaviour (in this case an error will occur because str.split expects the second argument to be an integer) 
-    if match is not None:  ​ +    matches = matches.union(split_str)  # this does NOT update the global variable, it only creates a temporary object 
-        matches = matches.union({match})  # this does NOT update the global variable +    return ​split_str 
-        return True + 
-    return ​False  +def process_matches(matches):​ # parameter ​names can also shadow outer variables or imports
-def process_matches(matches):​ # the parameter ​name shadows the global variable+
     for m in matches:     for m in matches:
-        m = get_some_value() ​ # this redefines the loop variable+        m = get_some_value() ​ # this redefines the loop variable, which may or may not be the desired outcome
         ...         ...
 </​code>​ </​code>​
 </​hidden>​ </​hidden>​
  
-<hidden function ​or method refefinitions and shadowing>​ +==== Excessive, unused ​or misplaced imports ====
-<code python>+
  
-def f(): +The imports should be placed at module-level,​ at the beggining of the file (not inside function or class definitions or within control blocks) ​and should ideally import either the module as an object (optionally renamed) or only the required objects from that module.
-  print(1) +
-   +
-def g(): +
-  x = 2 +
-  def f():  # this shadows the global function '​f'​ +
-     ​print(x) +
-  f() +
-   +
-def f():  # this is an error +
-  print(3) +
- +
-class A: +
-    def g(self): +
-        print(6) +
-    def __init__(self):​ +
-        self.g = 5  # this attribute hides the method g +
-    def f(self): # this is ok, as it is a method so it will never be confused with the outer '​f'​ +
-        print(4) +
-    def f(self): +
-        print(5) ​ # this is an error +
-</​code>​ +
-</​hidden>​ +
- +
-==== Import errors ==== +
- +
-The imports should be placed at module-level,​ at the beggining of the file (not inside function or class definitions or within control blocks)should ideally import either the module as an object (optionally renamed) or only the required objects from that module, should not cause import cycles, and should not contain additional errors+
  
 <hidden possible types of errors> <hidden possible types of errors>
-<code python ​incorrect_imports.py> +<code python>​ 
-import nonexistent_module ​ # module does not exist, remove this or replace with correct module +from time import *  # star imports are generally to be avoided. ​only import what is needed. for example, 'form time import perf_counter'​ 
-from time import *  # only import what is needed. for example, 'form time import perf_counter'​ +form typing import Generic # unused import, consider deleting this 
-form typing import Generic # unused import +from numpy import * # if you use many different things from a module, listing all imports may not be feasable. in this case, consider importing the module ​as an object rather than individual labels from it. (qualified import): '​import numpy' or '​import numpy as np'
-from re import DFA # no object called '​DFA'​ exists in re +
-from numpy import * # if you use many different things from a module, consider importing the module (qualified import): '​import numpy' or '​import numpy as np'+
 import pandas as pandas # useless renaming using '​as'​ import pandas as pandas # useless renaming using '​as'​
-import cyclic 
  
-... +def time_something(f,​ x): 
- +   start = perf_counter() 
-</code> +   ... # do some pandas/numpy processing on x, generating x_processed 
-<code python cyclic.py>​ +   f(x_processed) 
-import incorrect_imports # this will cause a cyclic import. python knows how to handle this most of the time, but it WILL cause some errors in certain situations+   end = perf_counter() 
 +   ​return end - start
 </​code>​ </​code>​
 </​hidden>​ </​hidden>​
Line 136: Line 121:
 </​hidden>​ </​hidden>​
  
 +==== Redundant and trivially simplifiable code ====
 +This includes:
 +  * unreachable code,
 +  * expressions with no effect
 +  * unused variables
 +  * redundant code and expressions written in an overcomplicated manner (using extra unneeded statements or harder-to-read versions of statements, such as dunders instead of builtins).
 +  * use of improper iteration methods (for example, iterating over the range of indices and then accessing a list directly in the loop, instead of iterating over the list)
 +<​hidden>​
 +<code python>
 +def f():
 +    x = 1  # this value is never used
 +    1+2  # this statement has no effect
 +    y = 1
 +    y = y # this statement is redundant
 +    return True
 +    y += 1  # this code is unreachable
 +
 +def g(z): # z is not used
 +    if 1 < 2:  # two errors: constant comparison (the condition always evaluates to True, so it can be safely replaced with True), and constant if condition (the '​then'​ branch is always executed, so the if can be removed)
 +       # the following if can be easily simplified to '​return f()' or '​return bool(f())'​ if the result is not guaranteed to be a bool but is interpreted as such
 +       if f():
 +           ​return True
 +       else:
 +           ​return False
 +
 +h = lambda x: x + 1 # use a more easily readable function def
 +
 +t = (lambda x: x * 2)(4) # do not define a lambda just to immediatly call it, use the expression 4 * 2 directly (or, in this case, just write 8)
 +
 +l = [1,2,3]
 +
 +if not not 1 in l: # double not is redundant
 +    print(not 1 < 2) # replace not less than with >=
 +
 +s = {x for x in l}  # do not use comprehensions,​ just use the built-in set() constructor
 +
 +for i in range(len(l)):​
 +    print(l[i]) # do not iterate over range, then index the list, just iterate over the list (use enumerate if you also need the index)
 +
 +def reurns_nothing():​
 +    print("​imagine this changed some state"​)
 +    return ​ # this return is not needed
 +    ​
 +while True:
 +    if some_condition(): ​ # use 'while some_condition():'​ instead
 +        break
 +    print("​do something"​)
 +
 +def my_min(ls): ​ # do not do this, python already has min, max, any, all, sum implemented (along with many others)
 +   m = ls[0]
 +   for x in ls[1:]:
 +       if x < m:
 +          m = x
 +   ​return x
 +
 +class A:
 +    pass
 +    ​
 +a = A()
 +
 +if a.__class__ == A: # use isinstance or match statements
 +    print(a.__str__()) ​ # use str(a)
 +</​code>​
 +</​hidden>​
 +
 +
 +==== Complexity ====
 +
 +Your functions and methods should be modular and it should be easy to understand their control flow. We will look at the following metrics: statements per function, number of branches and loops (cyclomatic complexity).
 +
 +Another side of this is that duplicate or very similar sequences of code should be refactored into functions or methods if possible.
 +
 +==== Return consistency ====
 +
 +All return statements of a function should return an exppression or none of them should
 +
 +<​hidden>​
 +<code python>
 +def f():
 +   if some_condition():​
 +       ​return True
 +   elif some_other_condition():​
 +       ​return # because the other return yields a value (True), this should explicitly return a value as well
 +   # if both conditions are false, an implicit bare return exists here, which should be replaced by a return with a value
 +</​code>​
 +</​hidden>​
 +
 +==== Dangerous default values ====
 +
 +You should be careful when using default values for function/​method parameters. Object values (such as list/​set/​dict literals) will be reused between calls and may lead to unexpected behaviour
 +
 +<hidden incorrect code>
 +<code python>
 +def f(l = [])
 +    l.append(1)
 +    print(l)
 +
 +f() # prints '​1'​
 +f() # prints '1 1'
 +f() # prints '1 1 1'
 +</​code>​
 +</​hidden>​
 +
 +<hidden possible fix>
 +<code python>
 +def f(l = None)
 +    if l is None:
 +        l = []
 +    l.append(1)
 +    print(l)
 +    ​
 +
 +
 +f() # prints '​1'​
 +f() # prints '​1'​
 +f() # prints '​1'​
 +</​code>​
 +</​hidden>​
 +
 +===== Improvement suggestions =====
 +
 +These are cases in which your code is readable and without major performance issues, but could be improved by using more specific or clear syntax.
 +
 +<note important>​
 +Having very few of these suggestions left unimplemented will grant you a bonus of up to 0.1 points per stage
 +(meaning you will be able to reach 1.1 points on each stage).
 +
 +This bonus is only granted if no penalty is given from error-prone practices.
 +</​note>​
 +
 +==== Comprehensions ====
 +
 +You can use comprehensions instead of for loops, such as the ones described [[lfa:​2023:​lab01#​list_comprehensions|here]] to more succintly perform operations resulting in lists, sets or dictionaries by iterating on other objects, especially maps, filters, cartesian products, and flattens.
 +
 +<hidden loop-based code>
 +<code python>
 +
 +L = [1,2,3,4]
 +S = {
 +    ['​a',​ '​b'​],​
 +    ['​c',​ '​d'​]
 +}
 +D = {'​a':​1,​ '​b':​ 2, '​c':​3}
 +
 +# maps
 +double_l = []
 +for x in L:
 +    l1.append(x * 2)
 +
 +reverse_d = {}
 +for k,v in D.items():
 +    reverse_d[v] = k 
 +
 +# filter
 +
 +even_l = []
 +
 +for x in L:
 +    if x % 2 == 0:
 +        even_l.append(x)
 +
 +# cartesian product
 +
 +all_pairs = []
 +for x in L:
 +    for y in S:
 +        all_pairs.append((x,​y))
 +        ​
 +# inner object iteration (flattening)
 +
 +flat_s = set()
 +
 +for l in S:
 +   for x in l:
 +        flat_s.append(x)
 +
 +# combined operations
 +
 +comb = []
 +
 +for x in L:
 +   if x % 2 == 0:
 +      for l in S:
 +          for y in l:
 +              comb.append(y * x)
 +
 +</​code>​
 +</​hidden>​
 +
 +
 +<hidden equivalent comprehensions>​
 +<code python>
 +
 +L = [1,2,3,4]
 +S = {
 +    ['​a',​ '​b'​],​
 +    ['​c',​ '​d'​]
 +}
 +D = {'​a':​1,​ '​b':​ 2, '​c':​3}
 +
 +double_l = [x * 2 for x in L]
 +
 +reverse_d = {v:k for k,v in D.items()}
 +
 +even_l = [ x
 +            for x in L
 +            if x % 2 == 0
 +         ]
 +         
 +all_pairs = [(x, y)
 +                   for x in L
 +                   for y in S
 +            ]
 +            ​
 +flat_s = { x
 +            for l in S
 +            for x in l
 +         }
 +
 +comb = [ x * y
 +               for x in L
 +               if x % 2 == 0
 +               for l in S
 +               for y in l
 +       ]
 +</​code>​
 +</​hidden>​
 +
 +==== Swapping variables ====
 +
 +In python you can use ''​a,​ b = b, a''​ to swap the values of 2 variables a and b instead of using temporary variables.
 +
 +==== Type hints ====
 +
 +Type hints can help others (including yourselves if you come back to the code after a small break) better understand your code, and allows software tools give you meaningful feedback (autocomplete/​intellisense,​ live type errors and suggestions). It is not necessary to give hints to every single variable you define, but it is recommended to use type hints on functions, methods (both parameters and return types) and class members, and use generics whenever applicable.
 +
 +
 +
 +===== Example code =====
 +
 +Here are some small examples of possible code submissions for a very simple task, one which is penalised, one which recieves regular points and one which also recieves a bonus.
 +
 +TODO: decide task and give example code
 +
 +
 +==== Penalised code ====
 +
 +<code python>
 +
 +</​code>​