万恶的"delete"，慎用数组的delete

    一直以来我的网站存在着一个令人很崩溃的问题。就是用户上传的图片会丢失。而且是连带相册一起丢失。开始还以为是什么地方误删了，冲突了。查了很久很久无果。关键是这个BUG并不是一定出现。而且，我测试时就从来没碰到过。
    昨天一个用户刚注册，开始上传图片。我就密切注意。过了一会儿，果不然，相册丢失了。很纳闷。打开production.log看了N久。没发现异常啊。注：里面我没加sql的log。太多了。
    但是这个问题很郁闷，而且已经郁闷很长一段时间了。这个用户还好。又继续新建相册，传照片。一会儿。又没了。这就郁闷了。我就不得不重新开了sql的log。果不然，确实有album destroy的记录。很奇怪。
     于是我备份了该用户消失的相册。（手快）自己回复数据来做测试。（因为我上传从来不出现。RP啊RP）
     通过看sql的log。和每个页面调用。（tail 就是好用啊）终于发现了居然是载入用户首页的时候相册被删除的。太奇怪了。怎么会这样。打开controller。里面有一句：

@photos = @lovetree.show_photos(current_user)

那肯定是该句的问题。遂打开Lovetree模型。找到show_photos。才恍然大悟！
由于用户主页回去把用户设了访问权限的相册移开。我用的方法是先找到所有属于该用户的相册。然后：

albums.each{|a| albums.delete(a) unless a.authorize(user)}

本来意思是如果用户设了权限，就把该相册从albums数组中删除掉。最后留下的就显示。结果造成了并不是数组中去删除相册，而是直接把该相册也删除了。
万恶的delete啊。最后改成albums - [a]就对了。
太痛苦的经历了。告诫大家一下！慎用数组delete。

评论

8 楼 xylffxyfpp 2009-02-16  

应该改成慎用each

7 楼 xylffxyfpp 2009-02-16  

a = [1,2,3,4,5,6]  
a.each do |x| a.delete(x) end 
puts a.inspect  # => [2,4,6]

是each的问题,用for就不会出错

6 楼 CaiDeHen 2009-01-09  

受教了，我也不知道当时为什么会在这个地方这样子用。其他地方也是用的select，这也是我之前为什么一直不知道为什么会出问题的地方。看来还是你们说的对，尽量避免使用有副作用的操作。

5 楼 wosmvp 2009-01-09  

感觉太乱了

为什么不在User模型中来个
named_scope :published,:conditions => ***
呢

控制器来个
current_use.albums.published
呢

4 楼 jiachengxi38 2009-01-09  

不知道楼主这样的逻辑想法从何而来，权限访问的思想好像在lZ这里有点怪异

3 楼 dennis_zane 2009-01-09  

这显然不应该采用有副作用的操作，select才是正道吧

2 楼 rubynroll 2009-01-09  

在Array#each里面删除Array的元素是一种极其危险的做法，你可以做一个实验：

a = [1,2,3,4,5,6]
a.each do |x| a.delete(x) end
puts a.inspect  # => [2,4,6]

楼主的场景应该用delete_if：

albums.delete_if do |a| not a.authorize(user) end

1 楼 hozaka 2009-01-09  

其实我觉得楼主是在绕了一个圈子。你的需求是选择出那些设置为公开的相册，而不是剔除那些设置为保密的相册。既然有现成的方法可以提供“选择”的功能，没必要再画蛇添足转两个弯。

两者的区别在于方法的不同，select 可以理解为仅仅是读的操作，即使被覆盖，充其量也是显示的数据不正确而已，因此属于安全的方法；而 delete 带有写操作的意味，如果被覆盖过，那么就有可能改变原有的数据，属于不一定安全的方法。这种区别尤其是在使用第三方扩展很多的时候会产生影响。

最后一点，LoveTree#albums 返回值严格来说不是一个单纯的数组，混入了 Associations 模块，可以参考一下源码