CODEWITHSUNDEEP
c#entity-frameworklinq
I Love Stackoverflow
I Love Stackoverflow

Reputation: 6868

Delete records with multiple ids based on condition

Below is my class :

 public partial class Ads
    {
       public int Id { get; set; }
       public int RegionId { get; set; }
       public string Name { get; set; }
       public int Group { get; set; }
    }

Records : 

Id      Name   Group
1       abc     1
2       xyz     1
3       lmn     1
4       xxx     2
5       ppp     2
6       ttt     3
7       ggg     3

Now I want to remove all records/only that record with particular id of same group for some ids.

Code : 

public void Delete(int[] ids,bool flag = false)
        {
            using (var context = new MyEntities())
            {
                context.Ads.RemoveRange(
                    context.Ads.Where(t => (flag ?
                   (context.Ads.Any(x => ids.Contains(x.Id) && x.Group == t.Group)) : false)));
                context.SaveChanges();
            }
        }

What I am trying to do is something like below :

If flag is false with ids=3,5 then
    I want to delete only records with Id=3,5
Else if flag is true with ids=3,5 then
    I want to delete records with Id=3,5 but all other records too of the group to which ids=3,5 belong to.
    Here id=3 belongs to group 1 so I want to delete all records of group1 i.e id=1,2 like wise ids=5 belongs to
    group 2 so I want to delete all records of group 2 i.e id=4.

Expected output for this last case(flag=true) :

Id      Name   Group
6       ttt     3
7       ggg     3

But I think that I haven't done this is a proper way, and there is some source of improvement in the query.

Note : ids[] will always contains ids from different group and that too highest ids from different group.

How can I to improve my query for both the cases(flag=true and false)?

Upvotes: 0

Views: 3731

Answers (5)

Robert
Robert

Reputation: 2523

To me it looks like you have two different deletes here.

In first case you are only deleting the ads with given ID and this is pretty straight forward.

In second case you are deleting the ads with given ID and all other ads that contain the group of the recently deleted Ads. So in this case instead of deleting the ads with given Id first why not actualy get distinct groups for these ID-s and than just delete the groups.

EDIT

You can do it like this.

using (var context = new TestEntities())
{
    if (!flag)
        context.Ads.RemoveRange(context.Ads.Where(a => ids.Contains(a.Id)));
    else
        context.Ads.RemoveRange(context.Ads.Where(a => context.Ads.Where(g => ids.Contains(g.Id)).Select(x => x.Group).Distinct().Contains(a.Group)));
    context.SaveChanges();
}

For the more complicated case I am trying to get distinct groups for given id-s. So for ID-s 3 and 5 I am selecting the groups and than I am doing distinct on the groups since it might happen that the id-s have the same group. Than I am fetching all the ads that have these groups. So for passed values of 3 and 5 I would get groups 1 and 2 which I would than use to get all the ads that have that group. That in turn would yield id-s 1, 2, 3, 4, and 5 which I would than delete.

EDIT 2

If the complexity of second Linq query bothers you than write a SQL query.

context.Database.ExecuteSqlCommand( 
        "DELETE Ads WHERE Group IN (SELECT Group FROM Ads WHERE Id IN(@p1, @p2))", new SqlParameter("@p1", ids[0]), new SqlParameter("@p2", ids[1]));

This should be extra performant rather than rely on EF which will delete it one by one.

Upvotes: 1

jitender
jitender

Reputation: 10429

What about 

var removeRecs=context.Ads.where(t => ids.contains(t.id))
if(flag)
removeRecs.AddRange(context.Ads.where(t=> removeRecs.Any(r =>t.groupId==r.Id)))
Ads.RemoveRange(removeRecs);

Upvotes: 2

grek40
grek40

Reputation: 13438

You should separate your tasks...

if (flag)
{
    groupIds = db.Ads.Where(x => ids.Contains(x.Id)).Select(x => x.Group).ToList();
    db.Ads.RemoveRange(db.Ads.Where(x => groupIds.Contains(x.Group)).ToList());
}
else
{
    db.Ads.RemoveRange(db.Ads.Where(x => ids.Contains(x.Id)).ToList());
}

Upvotes: 1

Lucian Bumb
Lucian Bumb

Reputation: 2881

 public void Delete(int[] ids, bool flag = false)
        {
            using (var context = new MyEntities())
            {
                var items = context.Ads.Where(x => ids.Any(a => x.Id == a));


                if (!flag)
                {
                    //flag=false --> delete items with Id in ids[]
                    context.Ads.RemoveRange(items);
                }
                else
                {
                    var groups = items.GroupBy(a => a.Group).Select(a => a.Key);

                    //flag=true --> delete all items in selected groups
                    context.Ads.RemoveRange(context.Ads.Where(x => groups.Any(a => x.Group == a)));
                }
                context.SaveChanges();
            }

Upvotes: 1

verbedr
verbedr

Reputation: 2270

Do not make it too hard for your self, not everything must/can be done in the where statement of a query. Also a general rule of thumb in a loop try to factor out all the constant values and checks. So try this:

    public static void Delete(int[] ids, bool flag = false)
    {
        using (var context = new MyEntities())
        {
            var query = context.Ads.AsQueryable();
            query = flag
              ? query.Where(x => context.Ads
                                   .Where(i => ids.Contains(i.Id))
                                   .Select(i => i.Group)
                                   .Contains(x.Group))
              : query.Where(x => ids.Contains(x.Id));

            context.Ads.RemoveRange(query);
            context.SaveChanges();
        }
    }

Upvotes: 1

Related Questions