0

Here's my example code:

#!/usr/bin/env python3

LIST_OF_UNITS = {}

class Unit():
    def __init__(self,name, value):
       self.name = name
       self.value = value

def create_new_unit(name, value):
    return Unit(name, value)

def add_new_unit(d, name, value):
    d[name] = Unit(name, value)
    return d

unit = create_new_unit('reactor1', 1)
LIST_OF_UNITS[unit.name] = unit

unit = create_new_unit('reactor2', 2)
LIST_OF_UNITS[unit.name] = unit

LIST_OF_UNITS = add_new_unit(LIST_OF_UNITS, 'reactor3', 3)

print(LIST_OF_UNITS)

LIST_OF_UNITS = add_new_unit(LIST_OF_UNITS, 'reactor3', 4)

print(LIST_OF_UNITS)

As you can see I have two ways of adding objects to the dictionary. Not yet sure which is the better. One may be more flexible for solving my problem. So, I've included them both.

I want to build a list of reactors and their properties.

For each reactor I create an object that will eventually contain the properties of that reactor (like it's volume, operation start and end times,etc.)

I'd like to prevent the (duplicate) creation of a unit. In the example the creation of 'reactor3' with value 4 should be avoided.

What would be the best way to do that. Inside the class, using one of the methods or somehow else?

Your insights are greatly appreciated.

Aran-Fey
  • 39,665
  • 11
  • 104
  • 149
Mausy5043
  • 906
  • 2
  • 17
  • 39

3 Answers3

1

Just make check if the item is already in the keys of the dictionary. Insert only if not already there.

def add_new_unit(d, name, value):
    if(name in d.keys()):
        print("The reactor information for {name} already recorded!".format(name = name))
        return d
    d[name] = Unit(name, value)
    return d
Deepak Saini
  • 2,810
  • 1
  • 19
  • 26
  • `if name in d:` is sufficient. No `.keys()` needed. – user2390182 Aug 25 '18 at 09:44
  • @schwobaseggl, yes, indeed. Any specific reason as to why not use it, though? – Deepak Saini Aug 25 '18 at 09:45
  • It feels more pythonic (also omitting the java-style parentheses). In Python2, `keys()` also returns a list and thus significantly worsens the membership check performance wise. While that has been remedied in Python3, `name in d` is still the more generally applicable. – user2390182 Aug 25 '18 at 09:54
1

If you change your code around some, you can store all created Units as classvariable inside Unit. The factory-methods shoult be classmethods and will auto-add/create your instances to it.

class Unit():
    UNITS = {} # shared btw. instances

    def __init__(self, name, value):
        self.name = name 
        self.value = value

    # nicer output
    def __repr__(self): return "{} - {}".format(self.name, self.value)    
    def __str__(self):  return repr(self) 

# this should be a classmethod instead, depending on your usage you might want to
# raise Errors instead of returning existing instances
def create_new_unit(name, value):
    # create if needed, else return the one already in
    # does not alter Unit.value if present
    u = Unit.UNITS.setdefault(name, Unit(name,value))
    if u.value != value:
        raise ValueError("Unit '{}' exists with different value".format(name))
    else:
        return u

# this should be a classmethod instead, depending on your usage you might want to
# raise Errors instead of returning existing instances    def add_new_unit(name, value):
    # create new unit or alter an existing Unit's value
    # you should rename the method accordingly
    u = Unit.UNITS.setdefault(name, Unit(name,value)) 
    u.value = value  # change it if called again 
    return Unit.UNITS

unit1 = create_new_unit('reactor1', 1) 

unit2 = create_new_unit('reactor2', 2) 

all_units = add_new_unit('reactor3', 3)

for u in Unit.UNITS:
    print(id(Unit.UNITS[u]),Unit.UNITS[u]) 

all_units = add_new_unit('reactor3', 4)

for u in Unit.UNITS:
    print(id(Unit.UNITS[u]),Unit.UNITS[u]) 

Output:

140125186245968 reactor1 - 1
140125186246024 reactor2 - 2
140125186246080 reactor3 - 3

140125186245968 reactor1 - 1
140125186246024 reactor2 - 2
140125186246080 reactor3 - 4 # changed by add_new_unit


# if create_new_unit(..) same named unit again with different value:
# ValueError: Unit 'reactor2' exists with different value 

Personally I would advice against creating multiple ways to instantiate new ones. And I would probably put the "factory methods" as @classmethods and not inside the normal program. That way all the housekeeping of Units is done by the Unit class itself, and you can centralize the logic where it belongs instead of having to add created Units inside your main program.

Suggested read for @classmethod: Meaning of @classmethod and @staticmethod for beginner?

Patrick Artner
  • 50,409
  • 9
  • 43
  • 69
0

If you add an entry with the same key to the dictionary, the item will be overwritten so you will lose 1 of the entries.

If you want to avoid that, you can check if the name is already in the key values of the dictionary.

How about this?

if name in LIST_OF_UNITS.keys():
   # raise an error
else:
   pass
gpopides
  • 718
  • 5
  • 13
  • 1
    No need for `keys()`. `x in dct` checks whether a key `x` is in `dct`. – user2390182 Aug 25 '18 at 09:43
  • 2
    Yes this works. I just thought adding keys() would be more helpful for a beginner to understand if the key already exists, but x in dct is more simple. – gpopides Aug 25 '18 at 09:46