ASP.NET Логика контроллера MVC — это слишком много?

#asp.net-mvc

#asp.net-mvc

Вопрос:

Мне просто интересно, сколько логики вы вкладываете в свои контроллеры? Я знаю, что они должны быть тонкими, но, похоже, вам нужно что-то добавить.

Пожалуйста, взгляните на мой контроллер и дайте мне знать, что бы вы сделали, чтобы изменить его. Спасибо!!

 Namespace Controllers
    Public Class ShopController
        Inherits ControllerBase

#Region "Members/Properties"

        Private ProductService As IProductService
        Private UnitOfWork As IUnitOfWork

#End Region

#Region "Constructor(s)"

        Public Sub New(ProductService As IProductService, UnitOfWork As IUnitOfWork)
            Me.ProductService = ProductService
            Me.UnitOfWork = UnitOfWork

        End Sub

#End Region

#Region "Methods"

        <HttpGet()>
        Function Index() As ActionResult
            Return View(New ShopViewModel With {
                        .Categories = Mapper.Map(Of IEnumerable(Of Category), IEnumerable(Of CategoryViewModel))(ProductService.GetCategories)
                    })

        End Function

        <HttpGet()>
        Function ListProducts(ID As String, CatID As Integer) As ActionResult
            Return View(New ShopViewModel With {
                        .Products = Mapper.Map(Of IEnumerable(Of Product), IEnumerable(Of ProductViewModel))(ProductService.GetProductsByCategoryID(CatID))
                    })

        End Function

        <HttpPost()>
        Function GetCartAsJson() As ActionResult
            Return New JsonResult With {.Data = ShoppingCart}

        End Function

        <HttpPost()>
        Function AddItemToCart(Model As ShoppingCartItemViewModel) As ActionResult
            Dim Item As ShoppingCartItem

            Item = ShoppingCart.GetItemBySku(Model.Sku)

            If (Item IsNot Nothing) Then
                Item.Quantity  = Model.Quantity
                UpdateCartItemPricing(Item)
            Else
                Dim Product = ProductService.GetProductDetailBySku(Model.Sku)

                Item = New ShoppingCartItem With {
                    .ProductID = Product.ID,
                    .Sku = Model.Sku,
                    .Name = Product.Name,
                    .Description = Product.ChildProducts(0).Name,
                    .Price = 0D,
                    .Quantity = Model.Quantity
                }

                ShoppingCart.AddItem(Item)
                UpdateCartItemPricing(Item)

            End If

            Return New JsonResult With {.Data = ShoppingCart}

        End Function

        <HttpPost()>
        Function UpdateCartItem(Model As ShoppingCartItemViewModel) As ActionResult
            Dim Item = ShoppingCart.GetItemBySku(Model.Sku)

            If (Item IsNot Nothing) Then
                If (Model.Quantity < 1) Then
                    ShoppingCart.DeleteItem(Item)
                Else
                    Item.Quantity = Model.Quantity
                    UpdateCartItemPricing(Item)
                End If
            End If

            Return New JsonResult With {.Data = ShoppingCart}

        End Function

        Private Sub UpdateCartPricing()
            For Each Item In ShoppingCart.Items
                UpdateCartItemPricing(Item)
            Next
        End Sub

        Private Sub UpdateCartItemPricing(Item As ShoppingCartItem)
            Item.Price = ProductService.GetPriceForSkuByQuantity(Item.Sku, Item.Quantity)
        End Sub

#End Region

    End Class
End Namespace
  

Обновить

Итак, я переработал ее в несколько предложений (спасибо, ребята!). Это то, что я придумал:

 Namespace Controllers
    Public Class ShopController
        Inherits ControllerBase

#Region "Members/Properties"

        Private ProductService As IProductService
        Private ShoppingCartService As IShoppingCartService
        Private UnitOfWork As IUnitOfWork

#End Region

#Region "Constructor(s)"

        Public Sub New(ProductService As IProductService, ShoppingCartService As IShoppingCartService, UnitOfWork As IUnitOfWork)
            Me.ProductService = ProductService
            Me.ShoppingCartService = ShoppingCartService
            Me.UnitOfWork = UnitOfWork

        End Sub

#End Region

#Region "Methods"

        <HttpGet()>
        Function Index() As ActionResult
            Return View(New ShopViewModel With {
                        .Categories = Mapper.Map(Of IEnumerable(Of Category), IEnumerable(Of CategoryViewModel))(ProductService.GetCategories)
                    })

        End Function

        <HttpGet()>
        Function ListProducts(ID As String, CatID As Integer) As ActionResult
            Return View(New ShopViewModel With {
                        .Products = Mapper.Map(Of IEnumerable(Of Product), IEnumerable(Of ProductViewModel))(ProductService.GetProductsByCategoryID(CatID))
                    })

        End Function

        <HttpPost()>
        Function GetCartAsJson() As ActionResult
            Return New JsonResult With {.Data = ShoppingCart}

        End Function

        <HttpPost()>
        Function AddItemToCart(Model As ShoppingCartItemViewModel) As ActionResult
            ShoppingCartService.AddItem(ShoppingCart, Model.Sku, Model.Quantity)
            Return New JsonResult With {.Data = ShoppingCart}

        End Function

        <HttpPost()>
        Function UpdateCartItem(Model As ShoppingCartItemViewModel) As ActionResult
            ShoppingCartService.UpdateItemQuantity(ShoppingCart, Model.Sku, Model.Quantity)
            Return New JsonResult With {.Data = ShoppingCart}

        End Function

#End Region

    End Class
End Namespace
  

Ответ №1:

Только действия AddItemToCart и UpdateCartItem требуют рефакторинга. Я бы перенес бизнес-логику, содержащуюся в этих двух методах, на уровень сервиса. Также частные методы в контроллере всегда вонючие.

Комментарии:

1. Не могли бы вы посмотреть мой комментарий выше к ответу Ахима, пожалуйста?

2. @Sam Striano, вы могли бы определить метод на IProductService , который позаботится об этой бизнес-логике. Затем просто вызовите их в действиях контроллера.

3. ОК. My ShoppingCart — это класс в приложении MVC, который, вероятно, должен быть в моей модели домена, чтобы все уровни имели к нему доступ? Дело в том, что в моем ControllerBase у меня есть свойство ShoppingCart , которое заполняется в OnActionExecuting (я загружаю его в сеанс и возвращаю обратно), поэтому каждый контроллер, который наследует, ControllerBase имеет доступ к этой корзине, поэтому я мог бы использовать метод ProductService like AddItemToCart(Cart As ShoppingCart, Item As ShoppingCartItem) , а затем передавать их из ShopController ?

4. @Sam Striano, да, наличие AddItemToCart метода на вашем уровне обслуживания, который принимает корзину покупок в качестве параметра, является одним из способов рефакторинга этого действия контроллера.

Ответ №2:

Тонкий не означает пустой! 😉 Следующая часть, на мой взгляд, немного «пахнет»:

 Dim Item = ShoppingCart.GetItemBySku(Model.Sku)

If (Item IsNot Nothing) Then
  If (Model.Quantity < 1) Then
    ShoppingCart.DeleteItem(Item)
  Else
    Item.Quantity = Model.Quantity
    UpdateCartItemPricing(Item)
  End If
End If
  

Я бы, вероятно, перенес это на ShoppingCard. Похоже на нарушение правила «рассказывать, не спрашивая».

Комментарии:

1. Я использую внедрение зависимостей (новичок), и поэтому этот контроллер использует ProductService для получения информации об элементах и их ценах и т.д. Итак, я подумал, что у ShoppingCart должна быть реализация IProductService , но знаете ли вы, как заставить внедрение конструктора работать с подобным элементом? Может быть, я сбился с пути?? Я использую Ninject для DI.

2. Я не понял вашего вопроса!? Зачем менять конструктор? Приведенный код запрашивает у ShoppingCart некоторую информацию, основанную на некотором артикуле (который является частью модели). Затем он принимает некоторые решения и работает с картой покупок. Эта логика должна быть перенесена в ShoppingCard. Метод может называться «updateQuantity» и может ожидать модель. Таким образом, вы не запрашиваете информацию у карточки покупателя, а указываете ему, что делать. Это хорошая идея: pragprog.com/articles/tell-dont-ask