C#•3w ago

Trying to learn EF Core

Am I doing this correctly?

public static void SaveData(ConfigurationData dataToSave) {

    // Open a connection to the database and save the data.
    using (var gameDatabaseConnection = new DatabaseConnection(Paths.GetGameDatabaseFilePath())) {

        ConfigurationData search = gameDatabaseConnection.Find<ConfigurationData>(dataToSave.PlayerProfileId);
        if (search is null) {
            // If the configuration data doesn't exist, create a new one.
            gameDatabaseConnection.ConfigurationDataSets.Add(dataToSave);
        } else {
            // If the configuration data exists, update it.
            gameDatabaseConnection.ConfigurationDataSets.Update(dataToSave);
        }

        // Save the changes to the database.
        gameDatabaseConnection.SaveChanges();

        // Update the cache with the new data. Updating inside the connection ensures the cache is
        // always up-to-date with the database. If the database operation fails, the cache won't be updated.
        Cache[dataToSave.PlayerProfileId] = dataToSave;
    }
}

public static void SaveData(ConfigurationData dataToSave) {

    // Open a connection to the database and save the data.
    using (var gameDatabaseConnection = new DatabaseConnection(Paths.GetGameDatabaseFilePath())) {

        ConfigurationData search = gameDatabaseConnection.Find<ConfigurationData>(dataToSave.PlayerProfileId);
        if (search is null) {
            // If the configuration data doesn't exist, create a new one.
            gameDatabaseConnection.ConfigurationDataSets.Add(dataToSave);
        } else {
            // If the configuration data exists, update it.
            gameDatabaseConnection.ConfigurationDataSets.Update(dataToSave);
        }

        // Save the changes to the database.
        gameDatabaseConnection.SaveChanges();

        // Update the cache with the new data. Updating inside the connection ensures the cache is
        // always up-to-date with the database. If the database operation fails, the cache won't be updated.
        Cache[dataToSave.PlayerProfileId] = dataToSave;
    }
}

42 Replies

PilvinenOP•3w ago

I'm not quite sure how to handle the cases where: 1) The entry does not yet exists. Create and save. 2) The entry exists. Update and save. I've been having some issues, including but not limited to:

System.InvalidOperationException: The instance of entity type 'ConfigurationData' cannot be tracked because another instance with the same key value for {'PlayerProfileId'} is already being tracked.

I think I understand Add correctly. But I'm not sure if Update does what I think it does.

Keswiik•3w ago

This means that you are attempting to use tracking queries for the same entity on two different dbcontexts

PilvinenOP•3w ago

Right. I kind of get that, but. How? Where? What's the concrete issue?

Keswiik•3w ago

The concrete issue is holding multiple instances of your DbContext in memory somewhere for example, say you decide to reuse a dbcontext you load some data with it, and it starts tracking PlayerProfile with an id of 5

PilvinenOP•3w ago

I thought I'm supposed to instantiate a new DbContext every time I want to access something from the database.

Keswiik•3w ago

yes, i am giving an example of how newcomers with EF tend to misuse it

PilvinenOP•3w ago

All right.

Keswiik•3w ago

but I also don't know why you're directly using a DatabaseConnection if you're trying to use EFCore

PilvinenOP•3w ago

It's what I call my class: https://gist.github.com/Pilvinen/fa99bc00be16431981373e9dec1542f7

Keswiik•3w ago

ah, I would suggest naming it <Something>DbContext personally as it also exposes a database connection under <dbContext>.Database.Connection

PilvinenOP•3w ago

Ah, okay. I will rename it to GameDbContext. I still didn't quite understand where I'm going wrong.

Keswiik•3w ago

Is this the only place where you're using your dbcontext? my guess is that you're not properly disposing of one somewhere but without seeing more code it is hard to tell

PilvinenOP•3w ago

Well, I'm using it in 22 different places, I believe.

Keswiik•3w ago

either that or you are running into an issue with concurrency and you are attempting to do two different things, with two different dbcontexts, which affect the same entity

PilvinenOP•3w ago

And they should all be contained inside using statement. It's not the Find which might be messing up with it? I'm not sure how to otherwise check if I need to Add or Update.

Keswiik•3w ago

personally, I use the presence of an ID field in my DTOs to determine which actions I need to take as you can do something like this to update a row without fetching it first, assuming you know it exists

var existingEntity = new SomeEntity() {
  IdProp = idOfKnownRecord
}

dbContext.attach(existingEntity)
existingEntity.SomeProp = aNewValue
dbContext.SaveChanges()

var existingEntity = new SomeEntity() {
  IdProp = idOfKnownRecord
}

dbContext.attach(existingEntity)
existingEntity.SomeProp = aNewValue
dbContext.SaveChanges()

PilvinenOP•3w ago

I used to have a singleton based approach where I kept the DbContext alive for the duration of the app. Because I'm using SQLite with EF Core. But I had weird issues where save took 30 ms. So I thought I'd give this a try if it works better by doing the whole "unit of work". But I'm still not sure what I'm doing wrong. I can't get it working at all. https://gist.github.com/Pilvinen/b9d17081c626e17ae57824428cda723e It can't be anything in here, right?

Keswiik•3w ago

No, it shouldn't be anything in there

PilvinenOP•3w ago

    public static void SetDeckBack(int value) {
        // Get the player profile id for the active profile.
        Guid playerProfileId = GameState.GetActivePlayerProfileId();
        // Get the configuration data from the database.
        ConfigurationData configurationData = ConfigurationDataService.LoadData(playerProfileId);

        // Modify the configuration data in the database data set.
        configurationData.DeckBack = value;

        // Save the changes to the database.
        ConfigurationDataService.SaveData(configurationData);
    }

    public static void SetDeckBack(int value) {
        // Get the player profile id for the active profile.
        Guid playerProfileId = GameState.GetActivePlayerProfileId();
        // Get the configuration data from the database.
        ConfigurationData configurationData = ConfigurationDataService.LoadData(playerProfileId);

        // Modify the configuration data in the database data set.
        configurationData.DeckBack = value;

        // Save the changes to the database.
        ConfigurationDataService.SaveData(configurationData);
    }

Here's one example how I call those methods. It fails on the SaveData call and inside the method specifically on the Update call.

Keswiik•3w ago

what does ConfigurationDataService.LoadData look like? but loading and saving using 2 different dbcontexts feels like an anti pattern it should be done with the same one otherwise you are throwing away EFs ability to use change tracking

PilvinenOP•3w ago

if (existingData is not null) {
    // If the configuration data already exists, update it.
    dbContext.Entry(existingData).CurrentValues.SetValues(dataToSave);
    // dbContext.ConfigurationDataSets.Update(dataToSave);
} else {
    // If the configuration data doesn't exist, create a new one.
    dbContext.ConfigurationDataSets.Add(dataToSave);
}

if (existingData is not null) {
    // If the configuration data already exists, update it.
    dbContext.Entry(existingData).CurrentValues.SetValues(dataToSave);
    // dbContext.ConfigurationDataSets.Update(dataToSave);
} else {
    // If the configuration data doesn't exist, create a new one.
    dbContext.ConfigurationDataSets.Add(dataToSave);
}

This works? Why? What's the difference?

Keswiik•3w ago

what was it before the change?

PilvinenOP•3w ago

Update(). The one I commented out there.

Keswiik•3w ago

my guess is that doing it this way prevents it from trying to load another instance of the entity, but I'm not sure I still wouldn't structure your methods this way

PilvinenOP•3w ago

I will try to inject the dbContext to get around not using a single context. As you suggested.

Keswiik•3w ago

it is more than just that, your method of loading the db context, returning it, making modifications, and then saving it using another dbcontext isn't very good

PilvinenOP•3w ago

Yes, that's what I meant. I will inject the dbContext to fix that.

Keswiik•3w ago

:PepoSalute:

PilvinenOP•3w ago

I mean like this:

public static void SetDeckBack(int value) {

    using (var dbContext = new GameDbContext(Paths.GetGameDatabaseFilePath())) {
        // Get the player profile id for the active profile.
        Guid playerProfileId = GameState.GetActivePlayerProfileId();
        // Get the configuration data from the database.
        ConfigurationData configurationData = ConfigurationDataService.LoadData(playerProfileId, dbContext);

        // Modify the configuration data in the database data set.
        configurationData.DeckBack = value;

        // Save the changes to the database.
        ConfigurationDataService.SaveData(configurationData, dbContext);
    }
}

public static void SetDeckBack(int value) {

    using (var dbContext = new GameDbContext(Paths.GetGameDatabaseFilePath())) {
        // Get the player profile id for the active profile.
        Guid playerProfileId = GameState.GetActivePlayerProfileId();
        // Get the configuration data from the database.
        ConfigurationData configurationData = ConfigurationDataService.LoadData(playerProfileId, dbContext);

        // Modify the configuration data in the database data set.
        configurationData.DeckBack = value;

        // Save the changes to the database.
        ConfigurationDataService.SaveData(configurationData, dbContext);
    }
}

So now they would share the same context. I was trying to get rid of database specific calls in my code. Which is why I was doing that in the first place.

Keswiik•3w ago

that is also generally considered an anti-pattern when it comes to EFCore

PilvinenOP•3w ago

But apparently it was not a good idea. Really?

Keswiik•3w ago

as the DbContext is your repository

PilvinenOP•3w ago

What's the proper way then?

Keswiik•3w ago

imo it really comes down to personal preference

PilvinenOP•3w ago

I mean, what options do I have?

Keswiik•3w ago

I'd put that entire method in your configuration service and let it handle all of the entity framework stuff then expose ConfigurationDataService.SetDeckBack which does everything

PilvinenOP•3w ago

Oh so I would just relay it forward .. or maybe combine Configuration.cs with ConfigurationDataService.cs 🤔 Because it's now starting to feel silly why it even exists separately.

Keswiik•3w ago

:Blob: takes a bit to get used to how to structure code using EF if all you've used is raw sql

PilvinenOP•3w ago

This is actually my first time ever using databases from C#. Which makes it even harder.

Keswiik•3w ago

once you figure out some of the small issues like this efc gets much easier to use

PilvinenOP•3w ago

But why is it an anti-pattern passing the dbContext as a parameter to a function?

Keswiik•3w ago

passing it in isn't as bad but depending on what you're doing there may be no point in creating the dbcontext externally

Gaming

Programming

Trying to learn EF Core