1

I have a class that I'm trying to create objects for with a given variable adjusted by a loop. I'm struggling to explain so perhaps an example will help. The actual class is more complicated than the below, but the general idea is the same. The below method works, but feels very wrong.

#!/usr/bin/python

class Cuboid:
  # default values for new instances
  width = 123
  height = 456
  depth = 789

  def __init__(self, var, val):
    exec("self." + var + " = val") # This feels un-pythony
  def area(self):
    return self.width*self.height*self.depth

param_list=[
['width',[5,10,15,20]],
['height',[5,10,15,20]]
]

for param_i in range(0,len(param_list)):
  for val_i in range(0,len(param_list[param_i][1])):
      thisCuboid = Cuboid(param_list[param_i][0],param_list[param_i][1][val_i])
      print(str(thisCuboid.area()))

This gives the expected output:

1798920
3597840
5396760
7195680
485235
970470
1455705
1940940

Apologies if the question is poorly worded, I'm quite new to python. Please feel free to suggest better wording/ask for clarification!

Note that the actual number of parameters that need changing is much more (something like a 20 dimensional cuboid to extend the analogy and the "area" depends on all 20 values), so the simple approach (create a list for each parameter and loop through the lists in turn, setting the named parameter accordingly) gets messy quickly (I need to add a loop for each of the parameters I want to change)

Dan
  • 242
  • 3
  • 11
  • 5
    Avoid using functions like `eval` and `exec`. No only are they dangerous, but there's almost always a better way. In your case, you can `settatr`: `setattr(self, var, val)`. – Christian Dean Jan 25 '18 at 01:10
  • 1
    Have you tried storing your parameters inside a dict? I'll see if I can work up an example. Also, this doesn't have anything to do with your problem, but isn't you area function actually returning the volume of the cuboid? – Charles Parr Jan 25 '18 at 01:11
  • 3
    Why not have `width`, `height`, and `depth`, simply be parameters of `__init__`? E.g. `def __init__(self, width=123, height=456, depth=789):` – Niema Moshiri Jan 25 '18 at 01:12
  • can you use `for param, values in param_list: for val in values: thisCuboid = Cuboid(param, val)`? – Harvey Jan 25 '18 at 01:17
  • Combine Charles Parr and Niema Moshiri's ideas: `def __init__(self, **params): self.values = { 'width': 123, 'height': 456, 'depth': 789} ; self.values.update(params)` Unless you really don't need that level of flexibility. – Harvey Jan 25 '18 at 01:20
  • I'm struggling to see how you'd pass a variable parameter name using that option. I also don't fully understand `self.values.update(params)` Would you mind elaborating more fully in an answer? – Dan Jan 25 '18 at 01:25

1 Answers1

0

Thanks to Christian Dean, the below works much better!

#!/usr/bin/python

class Cuboid:
  # default values for new instances
  width = 123
  height = 456
  depth = 789

  def __init__(self, var, val):
    setattr(self, var, val)
  def volume(self):
    return self.width*self.height*self.depth

param_list=[
['width',[5,10,15,20]],
['height',[5,10,15,20]]
]

for paramname, paramvals in param_list:
  for val in paramvals:
    thisCuboid = Cuboid(paramname, val)
    print(str(thisCuboid.area()))
Dan
  • 242
  • 3
  • 11
  • 1
    Side-note: `for i in range(len(x)):` where you only use `i` for indexing `x` is an anti-pattern. Iterate directly, and the code gets *much* cleaner (and faster too). `for paramname, paramvals in param_list:`, `for val in paramvals:`, `thisCuboid = Cuboid(paramname, val)` That's does the same thing as your nested loops at the bottom (aside from the `print`), and it's more succinct, self-documenting, and faster all at once. – ShadowRanger Jan 25 '18 at 01:24
  • Thank you! Edited accordingly! – Dan Jan 25 '18 at 01:28
  • 1
    I really don't understand why you're setting the variables like this instead of having `width`, `height`, and `depth` be parameters of `__init__` with default values, e.g. `def __init__(self, width=123, height=456, depth=789):`. It would be much cleaner – Niema Moshiri Jan 25 '18 at 01:29
  • In the actual code, I have about 40 default values of which I want to overwrite maybe 10, one at a time. Maybe I'm missing something though (I'm also probably guilty of using a "god class"!) I suppose I could have each of the parameters on a separate line still? I don't understand how I'd vary the parameter I want to change either (say I wanted to change depth instead of width for example)? – Dan Jan 25 '18 at 01:31
  • Ah, okay, I wasn't aware you had that many parameters. I think @Harvey's solution would be ideal, then – Niema Moshiri Jan 25 '18 at 02:13