作为一个开源的项目，尤其是用来推广 ruby 的项目，我觉得，应当要特别注意代码的质量。
我刚 fork 了 ruby-china 的 repo，第一眼就是看到了不少的 code smell。
单元测试我之前已经提过了。代码本身有不少地方需要改善，比如：
# 检查用户是否看过
# result:
# 0 读过
# 1 未读
# 2 最后是用户的回复
def user_readed?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
if uids.blank?
if self.last_reply_user_id == user_id || self.user_id == user_id
return 2
else
return 1
end
end
if uids.index(user_id)
return 0
else
if self.last_reply_user_id == user_id || self.user_id == user_id
return 2
else
return 1
end
end
end
这段代码里有不少问题：
1) 'readed' 不是一个英文单词，这样写看上去很怪 2) 0, 1, 2——这个比使用 a, b, c 做 variable 名称还糟糕。请使用 symbol。 3) 多层的 if 嵌套 4) 多个 return
我会慢慢 refactor 一些代码然后提交 pull request。希望大家也都可以注意一下代码的质量啦！抛砖引玉，多多包涵～～ :P
我的做法，拆解成多个 if，以上面代码为例
def user_readed?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
if uids.blank?
if self.last_reply_user_id == user_id || self.user_id == user_id
return 2
else
return 1
end
end
if uids.index(user_id)
return 0
end
if self.last_reply_user_id == user_id || self.user_id == user_id
return 2
end
1
end
再改进点应该，不过这个或许太过头了：
def user_readed?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
return [self.last_reply_user_id,self.user_id].include?(user_id) == true ? 2 : 1 if uids.blank?
return 0 if uids.include?(user_id)
return 2 if [self.last_reply_user_id,self.user_id].include?(user_id)
1
end
def user_readed?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
result = 1
if uids.blank?
result = [self.last_reply_user_id,self.user_id].include?(user_id) ? 2 : 1
elsif uids.index(user_id)
result = 0
end
result = 2 if [self.last_reply_user_id,self.user_id].include?(user_id)
result
end
是否读过，与是否最后一个回复者，为什么会在一个函数里判断呢？
最后一个回复者，肯定是已读者吧。
def user_read?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
if self.last_reply_user_id == user_id
return true
elsif uids.blank?
return false
elsif uids.index(user_id)
return true
else
return false
end
end
def is_last_reply?(user_id)
self.last_reply_user_id == user_id
end
不知道这样的代码是不是合适一些？
也许这样是更好的：
def user_read?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
result = false
result = true if !uids.blank? and uids.include?(user_id)
end
def is_last_reply?(user_id)
[self.last_reply_user_id, self.user_id].include?(user_id)
end
先对代码其他地方做了些微调整： https://github.com/huacnlee/ruby-china/pull/35/files
出去跑一圈步回来后继续重构……
基于 Topic id 来 cache 用户的阅读记录在高并发下面会出现写竞争冲突，改成基于 User id 来 cache 用户阅读过帖子的最后一个 reply id，不仅可以解决高并发问题，还能让代码实现更加简单。
#13 楼 @quakewang 赞同，在目前的场景下基于 User 来 cache 确实更好，不仅更符合自然思维且可以解决高并发问题。但如果仅仅使用 User 来 cache 当日后有“根据 Topic 来寻找所有读过的人”这样的需求时就无能为力了。
#13 楼 @quakewang 赞，不过这样还有那个绿色（2 最后是用户的回复）的状态无法实现
class User
def read_topic?(last_reply_id)
reply_ids = Rails.cache.read("user:#{self.id}:topic_read")
reply_ids.include?(last_reply_id)
end
def read_topic(last_reply_id)
reply_ids = Rails.cache.read("user:#{self.id}:topic_read")
reply_ids = [] if reply_ids.blank?
reply_ids.dup
reply_ids << last_reply_id
Rails.cache.write("user:#{self.id}:topic_read",reply_ids)
end
end
Topic#user_readed? 初步重构完毕：https://github.com/fredwu/ruby-china/commit/5f02b50749f6df945e1478f2106e78eeb68ce51d
重构需要一步一步慢慢来。
看了 @fredwu 的提交后被启发了，有了这个更精简的：
def user_read?(user_id)
user_id.in?(Rails.cache.read("Topic:user_read:#{self.id}") || [])
end
def is_last_reply?(user_id)
user_id.in?([self.last_reply_user_id, self.user_id])
end
另外，其实刚开始开发这个社区系统就用上那么多 cache，本身也是一个 smell (premature optimisation). :P 咳咳，我比较洁癖一点，哈，见谅见谅～～
我覺得這裡面一些設定是去撈 db(MongoDB) 的，本身就會比較慢。作 cache 不是正解，而是在設計時就要找出容易 slow 的地方做成類似 constant 的方式才會快
原理： 用 user_id + topic_id 作为 key，长期没有被访问的冷数据将会被 Memcached 自动挤掉，仅留下访问比较频繁的那些，此外存放只有一个 last_reply_id 有更新自动就过期了，无需手动清除
我不知道是不是 MongoDB 設置可能本身沒有被優化的關係 (?) 之前下載 zheye.org 這個 project 也是巨慢。
慢到最後我懶得找原因，把所有 db backend 都換成 mysql ....快速無比 T_T
Rendered home/index.html.erb within layouts/application (15.2ms) Completed 200 OK in 365ms (Views: 19.9ms | Mongo: 0.0ms)
def user_readed?(user_id)
uids = Rails.cache.read("Topic:user_read:#{self.id}")
foo = self.last_reply_user_id == user_id || self.user_id == user_id
return foo ? 2 : 1 if uid.blank? || !uids.index(user_id)
return 0
end
開發環境，我刻意關掉 memecached。就看得到速度低落了。
而且其實我還是相當吃驚的。因為我這台是 iMac 8 核 i5 8G，全新安裝.... 蠻確定不是我環境的問題。因為公司的 project 一樣都還是很快...
我剛剛看了很多代碼，測了一下，發現其實很多地方都是 LOGIC IN VIEW，這也可能是緩慢的元兇，因為 View 都是 eval 出來的....
我會慢慢把這一些抽的比較好維護，順便看能不能加速....
