patternpythonMinor
Review for Python mapping container in which every element is a key
Viewed 0 times
containereverypythonforelementwhichmappingreviewkey
Problem
I wanted to have a python container in which every element is a key (like in a set), but still supports mapping (like a dict) between the elements.
To get a container which supports this idea the elements have to be unique like in a set and the mapping between the elements has to be bidirectional. I couldn't find such thing so I tried to implement it myself.
As this is the first data structure I implemented myself in Python I would like to ask if anybody has comments or advices for me.
EDIT: updated version following the recommendations of @Winston and added a better example
```
import copy
class Network(object):
"""
A network is a container similar to a set in which
the elements can be mapped to each other (be connected).
The Elements in a network are called entities and are
unique in the network.
Single entities can be inserted into the network by
using the method insert(entity).
Connections between entities are made using the method
connect(entity1, entity2), which automatically inserts
the entity/-ies into the network if not existent beforehand.
Calling network[entity] returns a subset of the network,
containig all the entities connected to the given entity.
Removing an entity from the network results in removing
all its connections in the network aswell.
"""
def __init__(self):
self._db = {}
def __delitem__(self, entity):
self.remove(entity)
def __getitem__(self, entity):
"""
Returns a subset of the network, containig all the
entities connected to the given entity
"""
return self._db.__getitem__(entity)
def __iter__(self):
return self._db.__iter__()
def __len__(self):
"""Returns the number of entities in the network"""
return self._db.__len__()
def __repr__(self):
return self._db.__repr__()
def __str__(self):
return self._db.__str__()
def are_connected(self, en
To get a container which supports this idea the elements have to be unique like in a set and the mapping between the elements has to be bidirectional. I couldn't find such thing so I tried to implement it myself.
As this is the first data structure I implemented myself in Python I would like to ask if anybody has comments or advices for me.
EDIT: updated version following the recommendations of @Winston and added a better example
```
import copy
class Network(object):
"""
A network is a container similar to a set in which
the elements can be mapped to each other (be connected).
The Elements in a network are called entities and are
unique in the network.
Single entities can be inserted into the network by
using the method insert(entity).
Connections between entities are made using the method
connect(entity1, entity2), which automatically inserts
the entity/-ies into the network if not existent beforehand.
Calling network[entity] returns a subset of the network,
containig all the entities connected to the given entity.
Removing an entity from the network results in removing
all its connections in the network aswell.
"""
def __init__(self):
self._db = {}
def __delitem__(self, entity):
self.remove(entity)
def __getitem__(self, entity):
"""
Returns a subset of the network, containig all the
entities connected to the given entity
"""
return self._db.__getitem__(entity)
def __iter__(self):
return self._db.__iter__()
def __len__(self):
"""Returns the number of entities in the network"""
return self._db.__len__()
def __repr__(self):
return self._db.__repr__()
def __str__(self):
return self._db.__str__()
def are_connected(self, en
Solution
class network(object):The python style guide recommends CamelCase for class names
"""
A network is a container similar to a set in which
the elements can be mapped to each other (be connected).
The Elements in a network are called entities and are
unique in the network.
Single entities can be inserted into the network by
using the method insert(entity).
Connections between entities are made using the method
connect(entity1, entity2), which automatically inserts
the entity/-ies into the network if not existent beforehand.
Calling network[entity] returns a subset of the network,
containig all the entities connected to the given entity.
Removing an entity from the network results in removing
all its connections in the network aswell.
"""
def __init__(self):
self.db = dict()I might call this _db to make it clear that it is private. Empty dictionaries are usually created with
{}def __delitem__(self, entity):
self.remove(entity)
def __getitem__(self, entity):
"""
Returns a subset of the network, containig all the
entities connected to the given entity
"""
return self.db.__getitem__(entity)
def __iter__(self):
return self.db.__iter__()
def __len__(self):
"""Returns the number of entities in the network"""
return self.db.__len__()
def __repr__(self):
return self.db.__repr__()
def __str__(self):
return self.db.__str__()
def are_connected(self, entity1, entity2):
"""Test for the presence of a connection between entity1 and entity2."""
return entity1 in self.db[entity2] and entity2 in self.db[entity1]Do you actually need to check both?
def clear(self):
self.db.clear()
def connect(self, entity1, entity2):
"""
Connects entity1 and entity2. Automatically inserts
the entity/-ies into the network if not existent beforehand.
"""
self.db.setdefault(entity1, set()).add(entity2)
self.db.setdefault(entity2, set()).add(entity1)
def connect_many(self, entity, seq):
"""Connects entity with all entities in seq."""
for e in seq:
self.connect(entity, e)
def copy(self):
return self.db.copy()
def entities(self):
"""Returns a set of the entities in the network."""
return set(self.db.keys())Why are you returning this as a set? It seems to break symmetry with everything else a dict returns which are lists or generators.
def get(self, entity, default=None):
return self.db.get(entity, default)
def insert(self, entity):
"""Inserts an entity into the network."""
self.db.setdefault(entity, set())
def isolate(self, entity):
"""Removes all connections of an entity"""
for e in self[entity]:
if e != entity:
self[e].remove(entity)
self.db[entity].clear()
def pop(self, entity):
temp = self[entity]
self.remove(entity)
return temp
def remove(self, entity):
"""Removes an entity and all its connections from the network."""
for e in self[entity]:
if e != entity:
self[e].remove(entity)
self.db.__delitem__(entity)Why not
del self.db[entity]?def setdefault(self, entity):
return self.db.setdefault(entity, set())You are deviating from the dict interface here. Do you have a good reason?
def unique_mapping(self, entity):
"""If the given entity is mapped to a single entity this entity is returned"""
if len(self.db[entity]) == 1:
return next(iter(self.db[entity]))You may want to avoid fetching self.db[entity] multiple times
else:
return FalseYou should throw an exception not return a sentinel value. If you insists on returning a sentinel value use None.
def update(self, netw):
"""Updates the network with another given network. Equal entities are merged."""
for entity in netw:
for e in netw[entity]:
self.connect(entity,e)You could have this class inherit from dict, and then you'd be able to avoid all of the
def foo(self):
return self.db.foo()But then you might get methods you don't want as well. Thats a judgment call.
Code Snippets
class network(object):"""
A network is a container similar to a set in which
the elements can be mapped to each other (be connected).
The Elements in a network are called entities and are
unique in the network.
Single entities can be inserted into the network by
using the method insert(entity).
Connections between entities are made using the method
connect(entity1, entity2), which automatically inserts
the entity/-ies into the network if not existent beforehand.
Calling network[entity] returns a subset of the network,
containig all the entities connected to the given entity.
Removing an entity from the network results in removing
all its connections in the network aswell.
"""
def __init__(self):
self.db = dict()def __delitem__(self, entity):
self.remove(entity)
def __getitem__(self, entity):
"""
Returns a subset of the network, containig all the
entities connected to the given entity
"""
return self.db.__getitem__(entity)
def __iter__(self):
return self.db.__iter__()
def __len__(self):
"""Returns the number of entities in the network"""
return self.db.__len__()
def __repr__(self):
return self.db.__repr__()
def __str__(self):
return self.db.__str__()
def are_connected(self, entity1, entity2):
"""Test for the presence of a connection between entity1 and entity2."""
return entity1 in self.db[entity2] and entity2 in self.db[entity1]def clear(self):
self.db.clear()
def connect(self, entity1, entity2):
"""
Connects entity1 and entity2. Automatically inserts
the entity/-ies into the network if not existent beforehand.
"""
self.db.setdefault(entity1, set()).add(entity2)
self.db.setdefault(entity2, set()).add(entity1)
def connect_many(self, entity, seq):
"""Connects entity with all entities in seq."""
for e in seq:
self.connect(entity, e)
def copy(self):
return self.db.copy()
def entities(self):
"""Returns a set of the entities in the network."""
return set(self.db.keys())def get(self, entity, default=None):
return self.db.get(entity, default)
def insert(self, entity):
"""Inserts an entity into the network."""
self.db.setdefault(entity, set())
def isolate(self, entity):
"""Removes all connections of an entity"""
for e in self[entity]:
if e != entity:
self[e].remove(entity)
self.db[entity].clear()
def pop(self, entity):
temp = self[entity]
self.remove(entity)
return temp
def remove(self, entity):
"""Removes an entity and all its connections from the network."""
for e in self[entity]:
if e != entity:
self[e].remove(entity)
self.db.__delitem__(entity)Context
StackExchange Code Review Q#5185, answer score: 5
Revisions (0)
No revisions yet.