Open Party App是我们在为OpenParty开发的新网站，上面集成了会员注册报名参加活动等功能。我们把他放在Google Code上面以开源crowd sourcing的方式开发。Django我是业余水平，在这个项目里面继续磨练。上周末正好有空开发，顺便重构了一下代码，我把其中发现的一些bad smell纪录下来，给和我们一样的Django业余团队参考。
- {% ifequal event.is_upcoming 1%}
这是错误的，本意是{% if event.is_upcoming %}，因为它本身就是个Boolean
- <a class=”gray_link”>看看谁参加了活动</a>
gray_link是一个为样式命名的css class，但是css class的名字应该是一个结构语义的东西。也就是说css class应该是What it is而不是How it looks like。可以命名为<a class=”who-participated”>看看谁参加了活动</a>
- views.py里面return render_to_response(‘core/topic.html’, locals(), context_instance=RequestContext(request))
使用locals()我上次就发过邮件，这样做是不好的。因为locals会鼓励你创建很多没有意义的本地变量，将它们一股脑的传递到template。造成template里面由很多的判断逻辑，但是这些逻辑本身应该是在models层搞定的。当你从template重构这些晦涩复杂的判断逻辑的时候，你会发现问题可能就出在views.py里面那一个locals()。没错，我说的就是topics.html这个模版，看看判断是否可以投票的那部分逻辑，实在是太复杂了，我不得不给他写测试，花了不少的时间重构。在模版里面你总是会思考一些措辞和样式的问题，而忘记了分支逻辑的复杂，这种代码经常是有很多的bug的，而且它们非常难于维护和修复。一定要将这种复杂度封装在model层并进行测试才可以。
- 没有意义的判断
我看到很多if model.is_accepted == True: return True; else: return False这样的代码。我是在不理解为什么可能出现这样的代码……请注意这个和错误1差不多，都属于非常坏的重复代码。
- 测试
请注意，我们没有赶时间。我们不太需要临时性的代码，要随时注意代码的可维护性，因为我们是一个团队。测试就是代码的文档，请注意尽量尝试测试先行，如果不行也注意让你的model得到充分的测试。当你发现Event和Topic这两个Model的创建需要费这么多的周折的时候，你是否发现你的Model抽象有问题？你是否有重构的冲动，将它们的创建行为变得更简单。
- 命名
is_arranged这样的命名可能不太容易理解，注释上面写的是”’该话题是否已经加入到活动，并且活动尚未开始”’。那么如果keep stupid的话我觉得，is_arranged_in_coming_event是不是更容易理解呢？
is_shown，注释”’该话题所属活动是否正在进行或已经结束”’，这个就很难理解了。因为你看到“应该显示”这样一个名字的时候，知道后面的意思是活动正在进行或结束了么？is_arranged_event_started_or_closed也许更容易理解？
这都是一些重构建议，有一部分问题我已经重构过了。大家都在参加OpenParty的Coding Dojo，一起持续重构吧！