Rails 最佳实践:条件操作、多个操作还是方法?

发布于 2024-10-20 09:30:17 字数 1091 浏览 2 评论 0原文

我很想获得有关我最近正在处理的一段代码的一些意见。

我有一个模型,照片,有时(但并非总是)属于集合。我有一个供用户管理集合的页面,他们可以将任何未分​​配的照片添加到集合中或从集合中删除照片。

这是一个“编辑多个”的情况,因此我创建了两个新的控制器操作:select,它处理 GET 请求和视图,以及 assign,它处理来自的 PUT 请求选择视图中的复选框。

因为用户可以将照片添加到集合中,也可以从集合中删除照片,所以我的 assign 操作中有一个条件,如下所示:

def assign
    @photos = Photo.find(params[:photo_ids])
    case params[:assignment]
    when 'add'
        @photos.each do |p|
            p.collection = @collection
            p.save!
        end
        notice = "Photos added to collection."
    when 'remove'
        @photos.each do |p|
            p.collection = nil
            p.save!
        end
        notice = "Photos removed from collection."
    end
    redirect_to select_collection_photos_path(@collection), :notice => notice
end

这完全符合预期。然而,我对此感到不舒服,它似乎不符合“Rails Way”。

其他Rails开发者,当你们遇到这种情况时,你们会像我一样处理吗?您是否会将其拆分为两个控制器操作(即 add_to_collectionremove_from_collection),还是将其移动到模型中?如果你把它移到模型中,那会是什么样子?

我将不胜感激任何建议和反馈。谢谢!

I'm curious to get some input on a chunk of code I've been working on recently.

I have a model, photos, which sometimes (but not always) belong_to a collection. I have a page for users to manage a collection, they can add any unassigned photos to the collection or remove photos from the collection.

This is an "edit multiple" situation, so I created two new controller actions: select, which handles the GET request and the view, and assign which handles the PUT request from the checkboxes in the select view.

Because the user can be either adding photos to a collection or removing photos from a collection, my assign action has a condition in it, and it looks like this:

def assign
    @photos = Photo.find(params[:photo_ids])
    case params[:assignment]
    when 'add'
        @photos.each do |p|
            p.collection = @collection
            p.save!
        end
        notice = "Photos added to collection."
    when 'remove'
        @photos.each do |p|
            p.collection = nil
            p.save!
        end
        notice = "Photos removed from collection."
    end
    redirect_to select_collection_photos_path(@collection), :notice => notice
end

This works exactly as expected. However, I feel uncomfortable with it, it doesn't seem to fit the "Rails Way."

Other Rails developers, when you have this kind of situation, would you handle it as I have? Would you split this across two controller actions (ie add_to_collection and remove_from_collection), or would you move it to the model? If you were to move it to the model, what would that look like?

I'd appreciate any suggestions and feedback. Thanks!

如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。

扫码二维码加入Web技术交流群

发布评论

需要 登录 才能够评论, 你可以免费 注册 一个本站的账号。

评论(2

只是偏爱你 2024-10-27 09:30:17

可能有几种不同的方法可以重构它,但最明显的方法似乎是将所有照片逻辑移至照片模型。尽管这是您的照片控制器,但它不应该对照片模型了解太多。

我可能会在你的控制器中做一些类似的事情:

def assign
  Photo.update_collection(params, @collection)

  redirect_to select_collection_photos_path(@collection), :notice => "Photo collection updated"
end

然后在你的照片模型中:

class Photo < ActiveRecord::Base
  def self.update_collection(params, collection)

    photos = Photo.find(params[:photo_ids])

    case params[:assignment]
    when 'add'
      photos.each {|p| p.add_collection(collection) }
    when 'remove'
      photos.each {|p| p.remove_collection }
    end
  end

  def add_collection(collection)
    self.collection = collection
    save!        
  end

  def remove_collection
    self.collection = nil
    save!
  end
end

将功能分解为更小的模型方法可以更容易地进行单元测试,如果你不这样做,你应该这样做:)

There's probably a few different ways you could refactor this, but the most obvious one seems to be moving all the photo logic to the Photo model. Even though this is your photos controller, it shouldn't know that much about the Photo model.

I would probably do something along these lines in your controller:

def assign
  Photo.update_collection(params, @collection)

  redirect_to select_collection_photos_path(@collection), :notice => "Photo collection updated"
end

Then in your Photo model:

class Photo < ActiveRecord::Base
  def self.update_collection(params, collection)

    photos = Photo.find(params[:photo_ids])

    case params[:assignment]
    when 'add'
      photos.each {|p| p.add_collection(collection) }
    when 'remove'
      photos.each {|p| p.remove_collection }
    end
  end

  def add_collection(collection)
    self.collection = collection
    save!        
  end

  def remove_collection
    self.collection = nil
    save!
  end
end

Breaking the functionality up into smaller model methods makes it easier for unit testing, which you should be doing if you're not :)

窝囊感情。 2024-10-27 09:30:17

这实际上是accepts_nested_attributes_for 的主要候选者。

尽可能坚持标准 REST 约定,而不是考虑控制器中的新操作。除了花哨的 UI 显示内容(例如您的选择操作)之外,我很少发现需要偏离生成的scaffold_controller 中存在的标准 CRUD 操作。

如果您在照片模型中设置accepts_nested_attributes_for :collection,您应该能够构建一个特殊的表单,将集合分配给照片。我不会在这里详细介绍完整的细节,而是将您指向 http://railscasts.com/episodes/196-nested-model-form-part-1http://railscasts.com/episodes/197-nested-model-form-part-2 。视图中的工作量会更大,但您将在更简单、更易于测试的控制器和模型中遥遥领先。

This is actually a prime candidate for accepts_nested_attributes_for.

Instead of thinking about new actions in the controller, stick to the standard REST conventions whenever possible. Excepting fancy UI display stuff (like your select action), very rarely do I find that I need to deviate from the standard CRUD actions present in a generated scaffold_controller.

If you set accepts_nested_attributes_for :collection in your Photo model, you should be able to build up a special form that assigns collections to photos. I won't go into the full details here, but will instead point you to http://railscasts.com/episodes/196-nested-model-form-part-1 and http://railscasts.com/episodes/197-nested-model-form-part-2 . It'll be more work in the view, but you'll come out far ahead in more simple, easily testable controllers and models.

~没有更多了~
我们使用 Cookies 和其他技术来定制您的体验包括您的登录状态等。通过阅读我们的 隐私政策 了解更多相关信息。 单击 接受 或继续使用网站,即表示您同意使用 Cookies 和您的相关数据。
原文