0

I have the following input:

[['0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0'],
['0', '0', '10', '10', '0', '0', '10', '10', '0', '0', '0', '10', '10', '10', '10', '10', '0', '0', '0'],
['0', '0', '10', '10', '0', '0', '10', '10', '0', '0', '0', '0', '0', '0', '0', '10', '10', '0', '0'],
['0', '0', '10', '10', '0', '0', '10', '10', '0', '0', '0', '0', '0', '0', '0', '10', '10', '0', '0'],
['0', '0', '10', '10', '10', '10', '10', '10', '0', '0', '0', '0', '10', '10', '10', '10', '0', '0', '0'],
['0', '0', '0', '10', '10', '10', '10', '10', '0', '0', '0', '10', '10', '0', '0', '0', '0', '0', '0'],
['0', '0', '0', '0', '0', '0', '10', '10', '0', '0', '0', '10', '10', '0', '0', '0', '0', '0', '0'],
['0', '0', '0', '0', '0', '0', '10', '10', '0', '0', '0', '10', '10', '10', '10', '10', '10', '0', '0'],
['0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0'],
['0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0', '0']]

For the following function:

def assign_points(lists):
    """
    Assigns points from a list of lists to Vec4 objects.
    ...
    """
    z_max = float('-inf')
    z_min = float('inf')
    
    points = []
    for y, l in enumerate(lists):
        for x, point in enumerate(l):
            try:
                z, color = parse_point(point)
                if color is None:
                    color = no_color(z)
                points.append(Vec4(x, y, z, 0, color))
                
                # Update z_max and z_min if necessary
                if z > z_max:
                    z_max = z
                if z < z_min:
                    z_min = z

            except MyWarning as e:
                exit(f"Error on line {str(y)}, item {str(x)}: {e}")
    
    return points, z_max, z_min

However

My problem is that:

  1. I feel assign_points is too bloated and does/returns too much.
  2. the input lists can get quite big and I think looping trough them more than once is wasteful
  3. I'm not sure global vars is best practice

I thought about doing a function that stored the values and (depending on a flag as argument) returned the maximum values once called outside assign_points. But I'm pretty sure python doesn't have static vars (right?)

How would you go about this problem? Should I leave it as is or make global vars or do a 3rd option?

MiguelP
  • 416
  • 2
  • 12
  • 2
    *"python doesn't have static vars"* - that depends on what kind of "static vars" you are talking about. Different languages have slightly different meaning for those (with C and C++ having multiple different types of `static` variables, depending on what context they are used in) – UnholySheep Jul 27 '23 at 09:39
  • 1
    Instead of globals, consider classes - you will have methods and could keep state alongside it, recalculating only when needed – h4z3 Jul 27 '23 at 09:40
  • 1
    You should absolutely not use globals to return multiple values! See [here](https://stackoverflow.com/q/354883/509868) for options you have. – anatolyg Jul 27 '23 at 09:55

1 Answers1

1

As suggested in the comments, defining your own class would allow you to instantiate and manipulate objects to your liking.

Since there seem to be three relevant values (points, z_max and z_min) you could let your class inherit from namedtuple. assign_points can then be created as a method of your class:

from collections import namedtuple

class Vec4Obj(namedtuple('Vec4Obj', ['points', 'z_max', 'z_min'])):
    def __new__(cls):
        return super().__new__(cls, [], float('-inf'), float('inf'))

    def assign_points(self, lists: list):
        for y, l in enumerate(lists):
            for x, point in enumerate(l):
                try:
                    z, color = parse_point(point)
                    if color is None:
                        color = no_color(z)
                    self.points.append(Vec4(x, y, z, 0, color))
                    
                    # Update z_max and z_min if necessary
                    if z > self.z_max:
                        self.z_max = z
                    if z < self.z_min:
                        self.z_min = z

                except Warning as e:
                    exit(f"Error on line {str(y)}, item {str(x)}: {e}")

You can define other methods. For instance one that would take only one list and a y value to avoid looping over all lists, hence adding fewer points at once.

Using your class would look like:

my_obj = Vec4Obj() # instantiate with value Vec4Obj(points=[], z_max=-inf, z_min=inf)

my_obj.assign_points(lists) # add points to my_obj.points and set my_obj.z_min and my_obj.z_max
Tranbi
  • 11,407
  • 6
  • 16
  • 33