class MainPage(webapp.RequestHandler):
def get(self):
user = users.get_current_user()
tasks_query = Task.all()
tasks = tasks_query.fetch(1000)
if user:
url = users.create_logout_url(self.request.uri)
else:
url = users.create_login_url(self.request.uri)
template_values = {
'tasks': tasks,
'url': url
}
path = os.path.join(os.path.dirname(__file__), 'index.html')
self.response.out.write(template.render(path, template_values))
class Gadget(webapp.RequestHandler):
def get(self):
user = users.get_current_user()
tasks_query = Task.all()
tasks = tasks_query.fetch(1000)
if user:
url = users.create_logout_url(self.request.uri)
else:
url = users.create_login_url(self.request.uri)
template_values = {
'tasks': tasks,
'url': url
}
path = os.path.join(os.path.dirname(__file__), 'gadget.xml')
self.response.out.write(template.render(path, template_values))
views:
293answers:
4
+1
A:
Refactor for what purposes? Are you getting errors, want to do something else, or...? Assuming the proper imports and url dispatching around this, I don't see anything here that has to be refactored for app engine -- so, don't keep us guessing!-)
Alex Martelli
2009-07-24 00:44:23
There is much code repetition, can't you see? It's ugly, less readable and has poor maintanability.
Jader Dias
2009-07-24 01:14:39
removed "for AppEngine" for clarity purposes
Jader Dias
2009-07-24 01:15:32
The code is, indeed, copy-and-paste. If this is the concern, it would have been helpful to update the original question. If you feel that there is more refactoring needed beyond the repetition, you might get better answers if you indicated that too: the word "refactor" sets developers alight with thousands of notions that may not address the reason you feel that the code needs refactoring in the first place.
Jarret Hardie
2009-07-24 01:40:53
+1
A:
Since both classes are identical except for one string ('index.html' vs 'gadget.xml') would it be possible to make one a subclass of the other and have that one string as a class constant in both?
Jake
2009-07-24 01:20:01
+1
A:
Make it the same class, and use a GET or POST parameter to decide which template to render.
Emily Schloff
2009-07-24 01:22:22
Or use the version of get() that takes an extra parameter for the group matched in the regexp that dispatched it to the particular handler class. Then you don't have to clutter your URLs with parameters, but "blah/(.*)" passes 'index.html' to the handler's get method in one case, and 'widget.xml' in the other. You do have to remember to 404 any unrecognised values.
Steve Jessop
2009-07-24 02:14:58
+4
A:
Really it depends on what you expect to be common between the two classes in future. The purpose of refactoring is to identify common abstractions, not to minimise the number of lines of code.
That said, assuming the two requests are expected to differ only in the template:
class TaskListPage(webapp.RequestHandler):
def get(self):
user = users.get_current_user()
tasks_query = Task.all()
tasks = tasks_query.fetch(1000)
if user:
url = users.create_logout_url(self.request.uri)
else:
url = users.create_login_url(self.request.uri)
template_values = {
'tasks': tasks,
'url': url
}
path = os.path.join(os.path.dirname(__file__), self.template_name())
self.response.out.write(template.render(path, template_values))
class MainPage(TaskListPage):
def template_name(self):
return 'index.html'
class Gadget(TaskListPage):
def template_name(self):
return 'gadget.xml'
Steve Jessop
2009-07-24 01:40:09