Asp.net core separation of concern using service layer

Question

I'm having a problem on what is the best approach to design my service layer and use them in my controller. Here is my concern.

Currently I'm using this to delete categories

    [HttpPost]
    [ValidateAntiForgeryToken]
    public IActionResult Delete(List<Guid> ids)
    {
        if(ids == null || ids.Count == 0)
            return RedirectToAction("List");

        _categoryService.DeleteCategories(_categoryService.GetCategoryByIds(ids));
        _categoryService.SaveChanges();

        return RedirectToAction("List");
    }

my concern is should I just pass ids to DeleteCategories then call the GetCategoryByIds inside the DeleteCategories. And If I'm only going to delete 1 Category, is it better to add another method like DeleteCategory then in the controller check the length of the ids and if it is only 1, use DeleteCategory instead,

score 3 · Accepted Answer · answered Nov 29 '16 at 05:19

3

my concern is should I just pass ids to DeleteCategories then call the GetCategoryByIds inside the DeleteCategories.

Just pass the ID's to the DeleteCategories method. I wouldn't even bother calling GetCategoryByIds inside of it. There's no need to query the database for all the rest of the category information if you're just planning on deleting it.

And If I'm only going to delete 1 Category, is it better to add another method like DeleteCategory then in the controller check the length of the ids and if it is only 1, use DeleteCategory instead

I wouldn't bother with creating another method. You could just pass a list with one value in it. There's nothing a DeleteCategory method could do that you can't already do with DeleteCategories.

answered Nov 29 '16 at 05:19

Will Ray

10,621
3
46
61

Hi, I don't understand what you mean by _There's no need to query the database for all the rest of the category information if you're just planning on deleting it._ I'm using entity framework, how can I delete by not querying the database for the entity to delete? – markoverflow Nov 29 '16 at 05:33
1

@markoverflow As a general rule, it's good to avoid querying the database if you don't need to. Only the primary key should be needed to do a delete. There are a couple of examples in [this SO question](http://stackoverflow.com/questions/2471433/how-to-delete-an-object-by-id-with-entity-framework) on how to do it. – Will Ray Nov 29 '16 at 05:44

score 0 · Answer 2 · answered Nov 29 '16 at 06:17

It all depends on your business logic

If the user has the option to select more than one category and delete them at once, then it makes sense to have the delete method accepts a list of ID and delete them all, if the user can delete only one category, then it should be DeleteById(int categoryId).

As for calling GetCategoryByIds, still depends on your logic, if you have some kind of authorization, then you must retrieve the category first, make sure the user has an access to delete the provided category and proceed if everything is fine.

A note about your code, your service should have the logic for data manipulation encapsulated inside, you should not expose the SaveChanges method and give the control to the controller to call it, or at least implement the Unit of Work pattern if you need to implement some kind of transaction.

Asp.net core separation of concern using service layer

2 Answers2