4
\$\begingroup\$

I'm fairly new to Python and am exploring how to fix functions and make code 'more effective and readable'. As a first step, I would want to try to make shorter snippets of code here - for example by dividing it into more functions.

I would also appreciate further improvements regarding possible errors I might stumble upon.

No need for imports.

The code is supposed to take values from a batch on a x- and y-position in the unit circle.
Example input:

1, 0.1, 0.1, 10 
1, 0.2, 0.2, 15
2, 1.2, 1.2, 20
2, 0.5, 0.5, 25

Several values can be used in a batch. If x² + y² > 1, it will not take that value into account when calculating the average of a batch. If no values in a batch are used, no average is calculated (avoiding division by zero).

def a():
    while True:
        data = dict()  # Or data = {} 
        try:
            filename = input('Which data file? ') 
            with open(filename, 'r') as h:  
                for line in h:  
                    try:
                        four_vals = line.split(',')  
                        batch = four_vals[0]  
                        if not batch in data:  
                            data[batch] = []  
                        data[batch] += [  
                            (float(four_vals[1]), float(four_vals[2]), float(four_vals[3]))]  
                    except ValueError: 
                        print('Some data are not integers in batch',batch,", line", line,"\nThose values are not taken into account\n" )
        except IOError: 
            print('No such file or directory was found, the written file can not be opened.'"\n"'Look through misspellings or that the written file is actually in the pathway.')
            continue
        return data


def b(): 
    data = a()
    data = dict(sorted(data.items()))
    return data


def c():
    data = b()
    for batch, sample in data.items():
        try: #len(sample) > 0: 
            n = 0  # antal prov startar på 0
            x_sum = 0  # summan startar på 0
            for (x, y, val) in sample:  
                circle = x ** 2 + y ** 2
                if circle <= 1:  
                    x_sum += val  
                    n += 1  
                else:
                    print(batch,"\t","Outside of the unit circle,", circle,">1")
            average = x_sum / n  
            print(batch, "\t", average)
        except ZeroDivisionError: 
            print(batch, "\tNo data")
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Some clarification of the input file format would be good; I can derive it from the code, but it's not at all obvious. \$\endgroup\$ Dec 3 '21 at 2:34
5
\$\begingroup\$

Some good stuff:

  • You're using error-specific except clauses.
  • You're using where blocks.

Items you can improve:

  • Use a main method. See here for an example; in particular Example 2.

  • Don't nest function definitions unnecessarily. Instead of calling c, which calls b which calls a, pass their return values to each other:

    def main():
      c(b(a()))
    
  • Give your functions better names. Give-or-take the cost of verbosity, a function's name should tell you clearly what it does. When in doubt, give everything a clear whole-word name.

  • What's the point of b? If sorting the batches is important, leave a comment that explains why.

  • Consider learning comprehensions.

  • Until you learn classes and dataclasses (or NamedTuples), you can just use tuples to represent sequences of known length. This encodes your assumptions in the structure of the code.

  • Not that I like mutating values, but list has an append method; you don't have to say +=[item]. (It's more efficient too.)
    More broadly, it's always a good investment to read the official docs for whatever tools you're using.

  • Separate your user-input loop from your file-parsing process. This will probably require you to nest one function definition inside another:

    def a():
        while True:
            try:
                filename = input('Which data file? ') 
                with open(filename, 'r') as h:
                    return parse_file(h)
            except IOError: 
                print('No such file or directory was found, the written file can not be opened.'"\n"'Look through misspellings or that the written file is actually in the pathway.')
    
    def parse_line(line):
        batch, x, y, val = line.split(',')  # may raise if there's not the right number!
        return batch, tuple(float(i)  # should these all be floats? should batch be a string?
                            for i in (x, y, val))
    
    def parse_file(open_handle):
        data = {}
        for line in open_handle:
            try:
                batch, triple = parse_line(line)
                data.setdefault(batch, []).append(triple)
            except ValueError: 
                print('Some data are not integers in batch',batch,", line", line,"\nThose values are not taken into account\n" )  
        return data
    
  • Try to have all your validation in one place:

    def parse_line(line):
        b_, x_, y_, v_ = line.split(',')
        batch, x, y, val = strip(b_), float(x_), float(y_), int(v_)
        if (x**2) + (y**2) <= 1:
            return batch, (x, y, val)
        else:
            raise ValueError("How can you get the correct message to print out?")
    
  • If you do all your validation early, do you still need the try... except ZeroDivisionError? I have mixed feelings. On the one hand, it looks like it will no longer be able to happen. On the other, I don't want to encourage you to remove a failsafe.

  • Right now c has so many layers of indentation, maybe you should break out a separate function for handling a single batch. On the other hand, you can reduce the complexity by using more off-the-shelf stuff:

    def c(data):
        for batch, samples in data.items():
            n = len(samples)
            if n:
                total = sum(val for (x, y, val) in samples)
                average = total / n  
                print(batch, "\t", average)
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.