CODEWITHSUNDEEP
asp.net-mvcvb.netlinq-to-sqlforeach
LiamGu
LiamGu

Reputation: 5348

For Each Loop LINQ Insert giving me Cannot add an entity that already exists

I have the following code, that in my head should create a new row in a table:

Dim fin As Boolean
Dim db2 As New ChecklistModeldbmlDataContext
For Each a In bServers

   If collection("fin" & a.ServerName) = "true, false" Or collection("fin" & a.ServerName) = "true,false" Then
       fin = True
   Else
       fin = False
   End If

   bLog.AMLogID = amLog.LogID
   bLog.ByteCount = collection("bytes" & a.ServerName)
   bLog.DurationHours = collection("hours" & a.ServerName)
   bLog.DurationMinutes = collection("minutes" & a.ServerName)
   bLog.DurationSeconds = collection("seconds" & a.ServerName)
   bLog.IsFinished = fin
   bLog.ServerID = a.ServerID
   bLog.DetailsAndErrors = collection("details" & a.ServerName)

   db2.BackupLogs.InsertOnSubmit(bLog)
   db2.SubmitChanges()
Next

It only ever adds one record in to the table and then errors with Cannot add an entity that already exists.

Now it should enter 4 rows into the table, but I can't work out why the above gives me that error.

I've also tried having the db2.SubmitChanges() outside of the for each and it just inserts the last row.

Any thoughts?

Upvotes: 2

Views: 1243

Answers (4)

Konrad Rudolph
Konrad Rudolph

Reputation: 545915

Another thing about the code:

If collection("fin" & a.ServerName) = "true, false" Or collection("fin" & a.ServerName) = "true,false" Then
   fin = True
Else
   fin = False
End If

There are several things wrong with this code.

  1. First off, don’t write such If conditions that set a boolean variable. The condition already is the desired value. Instead, write it directly:

    fin = collection("fin" & a.ServerName) = "true, false" Or _
      collection("fin" & a.ServerName) = "true,false"

    This is true in general. So whenever you have code like this:

    If condition Then
    value = True
Else
    value = False
End If

    rewrite it as value = condition. Always. No exception. Or if the values are inverted, assign the negated result, i.e. value = Not condition.

  2. Secondly, never use And and Or in a boolean condition. These are bit operations and they are meaningless in a boolean condition. Sure, they yield the right result but this is rather coincidental.

    The correct solution is to use AndAlso and OrElse to join conditions. In your case, this would be:

    fin = collection("fin" & a.ServerName) = "true, false" OrElse _
      collection("fin" & a.ServerName) = "true,false"

    This is not only more logical, it’s also more efficient since the second part of the condition gets only evaluated if necessary, i.e. when the first part of the condition didn’t already evaluate to True (it’s the opposite for AndAlso).

  3. Lastly, why did you declare fin outside the loop when you only use it inside? Always try to put the declaration closest to its first use. As a consequence, this means that you should always initialize a value right away.

    So remove the declaration of fin from before your loop and correct the assignment inside:

    Dim fin As Boolean = collection("fin" & a.ServerName) = "true, false" OrElse _
                     collection("fin" & a.ServerName) = "true,false"

    If you have properly set up Visual Studio so that the VB project options Option Explicit, Option Strict and Option Infer are all On (you should do this!) the you can even omit the As Boolean in the declaration above because from the context it’s clear that fin must be a Boolean and the compiler correctly infers this.

Upvotes: 1

Kirschstein
Kirschstein

Reputation: 14868

You're not creating a new bLog in your loop. In each iteration, you're just modifying the data of a single record.

Try using the new keyword to create a new instance of bLog before you assign data to it in your ForEach loop

Upvotes: 3

Mike Two
Mike Two

Reputation: 46183

where is th instance of blog comming from? It looks like it is created outside the loop. Therefore on each pass you are modifying the instance of blog instead of adding a new one. Once the first one is saved it has an id and you can't call InsertOnSubmit on it again. You should be creating a new instance of whatever bLog is in each iteration.

Once you fix that you should only be calling SubmitChanges outside the loop

Upvotes: 0

Klaus Byskov Pedersen
Klaus Byskov Pedersen

Reputation: 120997

You should definitely new up bLog inside the For Each loop.

Upvotes: 0

Related Questions