C#•2mo 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•2mo 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•2mo ago

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

PilvinenOP•2mo ago

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

Keswiik•2mo 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•2mo ago

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

Keswiik•2mo ago

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

PilvinenOP•2mo ago

All right.

Keswiik•2mo ago

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

PilvinenOP•2mo ago

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

Keswiik•2mo ago

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

PilvinenOP•2mo ago

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

Keswiik•2mo 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•2mo ago

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

Keswiik•2mo 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•2mo 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•2mo 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•2mo 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•2mo ago

No, it shouldn't be anything in there

PilvinenOP•2mo 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•2mo 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•2mo 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•2mo ago

what was it before the change?

PilvinenOP•2mo ago

Update(). The one I commented out there.

Keswiik•2mo 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•2mo ago

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

Keswiik•2mo 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•2mo ago

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

Keswiik•2mo ago

:PepoSalute:

PilvinenOP•2mo 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•2mo ago

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

PilvinenOP•2mo ago

But apparently it was not a good idea. Really?

Keswiik•2mo ago

as the DbContext is your repository

PilvinenOP•2mo ago

What's the proper way then?

Keswiik•2mo ago

imo it really comes down to personal preference

PilvinenOP•2mo ago

I mean, what options do I have?

Keswiik•2mo 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•2mo 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•2mo ago

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

PilvinenOP•2mo ago

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

Keswiik•2mo ago

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

PilvinenOP•2mo ago

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

Keswiik•2mo 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