# Improvements in repository pattern [closed]

I am trying to learn the clean architecture by Uncle Bob. I wanted some basics and following this good post I tried to implement it in python with some minor differences.

class ValidationError(Exception):
pass

class Repository(object):

repositories = {}

@classmethod
def register(cls, type_, repo):
cls.repositories[type_] = repo

@classmethod
def get(cls, type_):
return cls.repositories[type_]

@classmethod
def all(cls):
return cls.repositories

class UserValidator(object):

def validate(self, user):
if not (user.name and user.email):
raise ValidationError

class UserRepo(object):

def __init__(self, validator=None):
self.users = {}
self.next_id = 1
self.validator = validator

def save(self, user):
self.validator.validate(user)
user.id = self.next_id
self.users[self.next_id] = user
self.next_id += 1
return user

def all(self):
return self.users

def get(self, id):
return self.users[id]

def delete(self, id):
return self.users.pop(id)

class CompanyRepo(object):

def __init__(self):
self.companies = {}
self.next_id = 1

def save(self, company):
company.id = self.next_id
self.companies[self.next_id] = company
self.next_id += 1
return company

def all(self):
return self.companies

def get(self, id):
return self.companies[id]

def delete(self, id):
return self.companies.pop(id)

class BaseEntity(object):

def __init__(self):
self.id = None
self.created_at = None
self.updated_at = None

class Company(BaseEntity):

self.name = name
super(Company, self).__init__()

class User(BaseEntity):

def __init__(self, name, email):
self.name = name
self.email = email
self.company_id = None
super(User, self).__init__()

def serialize(self):
return {'id': self.id, 'name': self.name, 'email': self.email, 'company_id': self.company_id}

def value(self):
return self.serialize()

def __init__(self, name, email):
self.user_repo = Repository.get('user')
self.company_repo = Repository.get('company')

self.company = self.company_repo.save(Company('FooBar', 'Dummy street'))
self.user = User(name, email)

try:
self.user.company_id = self.company.id
print self.user_repo.save(self.user).value()
print self.user_repo.all()
print self.user_repo.get(1)
print self.company_repo.all()
except ValidationError as ve:
print "Validation error!"

if __name__ == '__main__':

Repository.register('user', UserRepo(validator=UserValidator()))
Repository.register('company', CompanyRepo())



O/P from above code:

{'company_id': 1, 'email': 'foo@example.com', 'id': 1, 'name': 'Foo'}
{1: <__main__.User object at 0x7f4b34139d50>}
<__main__.User object at 0x7f4b34139d50>
{1: <__main__.Company object at 0x7f4b34139d10>}


I have basically tried to implement the in memory repository pattern. I would like to hear from others what they think, and why there is so scarcity in the python community regarding the design patterns and different architectures as compared to ruby?

PS: The code above is just for learning purpose hence error handling and error cases are not handled properly.

• Guys, I would like to remind again that its not a production application, I just tried to make a toy app and obviously it will have some missing features. Please try to remain stuck to the architect point of view like how will anyone make a persistence repository and maintain different DB related validations. Don't look into why I haven't used that function and all. Lets have a high level overview. – vivek May 26 '14 at 9:48
• This question is off topic. The actual question is very marginal because does not appear to be real code, but, it then asks for opinions about Python vs. Ruby. Then the comment above specifically asks for a review of the design, rather than the code. It wants opinions on a high level overview. This question is not on-topic for code review, the code is 'example' , not real code. – rolfl May 26 '14 at 10:37
• Hi guys, I want to know where this code belongs to? Obviously its not a production code but its not useless code too, it contains some important idea to maintain large code bases. – vivek May 26 '14 at 10:40
• Carefully read the on-topic criteria on Programmers, where, with some adjustment, this type of question would be on-topic. – rolfl May 26 '14 at 10:42

I'm skeptical of the value of this design pattern, for two reasons:

• The Repository class is basically just a Python dictionary. You never use Repository.all() anyway.
• Repository is actually a glorified namespace. It's a disguised mechanism for you to make a bunch of global variables.

Additionally, I see some puzzling aspects to the code:

• Why would UserRepo() make its .validator configurable? And why would it default to None?
• Since UserRepo and CompanyRepo contain nearly identical code, why not consolidate them? At the least they should have a common base class. Maybe they could even be handled by one class.
• The WorkerAdder object seems contrived. Why split the work between a WorkerAdder() constructor and an .add() method? Why is the company name and address hard-coded?

Some minor remarks:

• It's customary to chain to the superclass constructor first, not as the last statement in the subclass's constructor.
• Serialization means turning an object into a string representation; your User.serialize() returns a dictionary instead. The string representation needs to contain enough detail to reconstruct the object exactly; your User.serialize() omits the .created_at and .update_at timestamps.

In summary, Repository is just a dictionary that serves as a namespace for global variables. There's not much point to it, since symbol tables in Python are also dictionaries. UserRepo and CompanyRepo do serve a purpose, but you would have to reinvent the wheel (implementing the persistence mechanism, for example) to make them useful. You might as well use a real database (which could be as simple as ) with an ORM.

• Thanks for your useful insights but again I would like to remind you that its just an dummy implementation and not the full application, for eg: I have not implemented a full repository here, its just an example also, comments regarding a common base class is also not important here since that is not a main concern. Please check the intent and not the implementation. Also, I have updated the link to the blog post hope you will have a look at it once. – vivek May 26 '14 at 9:43
• I would argue against 200_success's argument that in-memory repository is useless, however @vivekpoddar's "This is not a real code" makes this question off-topic for CodeReview.SE and rendering any further discussion moot. – abuzittin gillifirca May 26 '14 at 10:29
• @abuzittin gillifirca so, it means if my code base is 2000 lines long then I have to paste the whole logic to get any idea. I am still wondering why anyone is not interested in bigger picture. This will help a lot of people specially those who wants to write decoupled code. At the end let me know if its off topic then where we need to throw this code? just saying its an off topic is not helpful either. – vivek May 26 '14 at 10:36
• The question was initially passable. I provided an answer that was both a code review and a design review. However, you then insisted that I ignore the code, thus making it a design review question and therefore off-topic. – 200_success May 26 '14 at 10:39
• @200_success, just let me know frankly did you have a look at clean architecture. You just bogged down to minor details in the code. See, you should sometime try to encourage good stuffs here specially if someone is trying to bring something good to the community. Did you ever talked about the service layer details or value object or anything. I don't wanted to know that Repository should be python dictionary no, it can't when you actually implement that. Or at last just try to implement by yourself and show us how it can be implemented properly – vivek May 26 '14 at 10:50