Entity Framework Core Respository Pattern - Duplication Check

Question

I am trying to work out a solution to check if any fields that should not be duplicated are having a row added that would break this rule. I am using the respository pattern so I wanted to create a method within the respository to do this.

In this example I am trying to add a company, in the controller before it's added it calls this method in the CompanyRespository:

    public bool Exists(Company company, bool ignoreId)
    {
        if (!ignoreId)
        {
            if (context.Companies.Any(c => c.Id == company.Id)) return true;
        }
        if (context.Companies.Any(c => c.TextId == company.TextId)) return true;
        if (context.Companies.Any(c => c.Email == company.Email)) return true;
        if (context.Companies.Any(c => c.PhoneNumber == company.PhoneNumber)) return true;
        return false;
    }

The issue I am facing is trying to return the error so the controller can send the error to the client. The obvious solution would be to just send Exceptions instead of returning true. However, if I was to call this just to say if there is a company with these parameters then I dont want exceptions sent, I just want a boolean in both cases.

The cut down question: Is it bad practice to in this situation create another method CheckDuplication() which would return exceptions instead of a boolean? If yes then what is the correct way for checking duplicated fields before saving to the database in Entity Framework Core?

Use a more complex return object to hold the desired information. Similar to how ModelState is used. — Nkosi, Dec 20 '17 at 15:09
I tend to agree with @Nkosi. Why not just return a model that has, say, two properties in it, one which is an IEnumerable or IList (whatever makes sense for you) and one which is a boolean indicating existence? [Edited for clarity.] — Trioj, Dec 20 '17 at 15:09
PS I dont think there is inherently a problem with separating existence checks and validation checks. It's fine to have an Exists() method that just returns boolean and some sort of Validate() method that either throws exceptions or returns some sort of enumerable of them. If it doesn't make sense to combine the two things into one model, then don't. Stick with your original plan. The downside to it is you are maintaining two methods every time you add a new field to the table. It's also possible that rolling this all into one method to return one object would actually make it clunkier. — Trioj, Dec 20 '17 at 15:19
mmm... why don't put ON DB (if you're using a SQL db ..not if you're using a MongDB ..) unique index or some check contraint? ..so when you try to save it ..EF do the check for you? — federico scamuzzi, Dec 20 '17 at 16:34
Think the two methods will be the way to go with the validate method returning the list with the exceptions to be parsed to the ModelState in the controller. I am already using the unique constraint but the only way I could find on the internet involved catching an exception followed by a lot of converting to different objects and clunky code and I wanted a cleaner solution. — Richard, Dec 20 '17 at 17:15

score 1 · Accepted Answer · answered Dec 21 '17 at 07:55

Having a separate Validation check would be fine, and allows you to write a more user friendly application. However don't make the mistake of relying on this check to ensure data integrity. Multi threading would be the main cause of concern. Sql DB can guard data integrity way better, so I would suggest defining unique constraints on each of these fields (for instance check out Setting unique Constraint with fluent API?). Of course that will raise EF exceptions but those can be caught and inspected to determine if they arer violations of the constraints or not. That way you could still return a boolean if required.

As a side note the Exits method can be written more efficiently by using an OR(||), that way you don't have to fire off so many sql queries.

I already had constraints enabled as I didn't want a chance of the database being in a bad state, if the database has a way to prevent a bad state then I'll primarliy use that. However, I was hoping for a user friendly solution which I feel is the validation method, just to return a easy to parse exception as opposed to inspecting a ef exception. Thanks for the side note! Did try something similar but didn't realise the "||" was the way to do it. — Richard, Dec 21 '17 at 16:24

Entity Framework Core Respository Pattern - Duplication Check

1 Answers1