-1
\$\begingroup\$

I know the difference between the Entity Framework connected mode vs disconnected mode. In connected mode we do all the stuff inside one single DbContext instance. In disconnected mode we do the stuff and then attach the Entity to a new DbContext instance.

My problem is that I - for a specific reason - had to create the DbContext instance globally in the form class without disposing it (I'm disposing the form after closing) and I'm confused and want to review my code and determine if it's a connected or a disconnected mode and if it's good practice to do this:

public partial class FrmProducts : MetroForm
{
    public FrmProducts()
    {
        InitializeComponent();
    }

    //The DbContext:      
    FDB.MFdb db = new FDB.MFdb();

    private void sfButton1_Click(object sender, EventArgs e)
    {
        try
        {
            //New row:
            if (txtID.Text.Trim() == "")
            {
                short maxID, newID;

                if (db.Products.Count() > 0)
                {
                    maxID = db.Products.Max(p => p.PID);
                    newID = ++maxID;
                }
                else
                    newID = 1;

                //New Database Entity:
                FDB.Product px = new FDB.Product();

                //Set entity data:
                px.PID = newID;
                px.P_Code = txtCode.Text;
                px.P_Name = txtName.Text;
                px.P_Purchase_Price = Convert.ToDecimal(txtPurchase.Text);
                px.P_Sale_Price = Convert.ToDecimal(txtSale.Text);
                px.P_Notes = txtNotes.Text;

                //Add entity to DbContext:
                db.Products.Add(px);

                db.SaveChanges();

                //This is a BindingSource Control:
                binSrc.Add(px);
            }
            else
            {
                //Edit row:
                int pid = Convert.ToInt16(txtID.Text);
                var row = db.Products.Single(b => b.PID == pid);
                row.P_Code = txtCode.Text;
                row.P_Name = txtName.Text;
                row.P_Purchase_Price = Convert.ToDecimal(txtPurchase.Text);
                row.P_Sale_Price = Convert.ToDecimal(txtSale.Text);
                row.P_Notes = txtNotes.Text;
                db.SaveChanges();
            }
            //Reset BindingSource to reflect updated data:
            binSrc.ResetBindings(false);
        }
        catch (Exception ex)
        {
            //Discard Db Changes if error occurred:
            foreach (var ent in db.ChangeTracker.Entries())
            {
                if (ent.State == EntityState.Modified)
                {
                    ent.State = EntityState.Unchanged;
                }
                else if (ent.State == EntityState.Added)
                {
                    ent.State = EntityState.Detached;
                }

            }
            MessageBox.Show(ex.Message + "\nInner Exception:\n" + ex.InnerException);
        }
    }
\$\endgroup\$

closed as unclear what you're asking by Gerrit0, t3chb0t, IEatBagels, Der Kommissar, Vogel612 Feb 28 at 23:28

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$ – Jamal Feb 25 at 4:00
  • \$\begingroup\$ in connected mode we do all the stuff inside one single DbContext instance -- Doesn't that answer you own question is it a connected or a disconnected mode? Whether or not it's good practice is opinion-based. \$\endgroup\$ – Gert Arnold Feb 25 at 13:37
  • \$\begingroup\$ @GertArnold I know that connected mode means that a single instance handles all data operation, but the famous practice is to use using { } and do all the stuff inside it. Here I didn't follow that practice and made the DbContext at form level which made me confused. Thanks for reply. \$\endgroup\$ – Ahmed Suror Feb 25 at 22:44
  • \$\begingroup\$ Working in connected mode in rich client applications is also normal. We can only give opinions here. \$\endgroup\$ – Gert Arnold Feb 26 at 8:42
  • \$\begingroup\$ Judging whether or not disconnected mode is appropriate in a given situation requires a lot of context. As it stands you're just stating "for reasons" in the question. That does not constitute sufficient context to meaningfully review the code you presented or even answer your question (which no answer on code review is obligated to in the first place)... \$\endgroup\$ – Vogel612 Feb 28 at 23:29
2
\$\begingroup\$

It is a best-practice to have a clear separation between UI layer (the forms) and data access, so all your data access logic (opening the connection, issuing quries, closing it etc.) should be handled in a separate class (service) that can be reused by other classes / forms.

This will help you have a single place to handle specific things like logging, reverting changes on error:

catch (Exception ex)
{
    //Discard Db Changes if error occurred:
    foreach (var ent in db.ChangeTracker.Entries())
    {
        if (ent.State == EntityState.Modified)
        {
            ent.State = EntityState.Unchanged;
        }
        else if (ent.State == EntityState.Added)
        {
            ent.State = EntityState.Detached;
        }
    }
}

Also, EF contexts are typically used for a short period (new + query stuff + save changes + dispose) because a Dispose does not mean a connection close (in most cases) since connection pooling kicks in. So, there is really no significant penalty, but you make sure that there no undisposed context lurking around.

There might be exceptions to this, such as when using a unit of work pattern which uses a connection per "unit of work" (e.g. thread, request), but stick the above for the beginning and you will be fine.

Also, try to create separate functions for each semantic part. E.g.: create new entity based on row, update entity based on row.

As a conclusion:

  • move all database context logic into a separate class
  • put all context related logic into a using block that ensures context disposal
\$\endgroup\$
  • \$\begingroup\$ The specific reason for not using Using { } is that I'm binding a DataGridView using a BindingSource to the same global DbContext to make sure that DbContext see all modifications made to the BindingSource, and if I used two different instances of DbContext the changes to BindingSource don't commit to the original data source and vice versa... \$\endgroup\$ – Ahmed Suror Feb 25 at 22:52
  • \$\begingroup\$ @AhmedSuror - yes, this is a valid scenario that I have not used in many years, as I typically work in 3-tier architecture (UI talks to an application server / REST API only and cannot see the database at all). \$\endgroup\$ – Alexei Feb 26 at 4:59
  • \$\begingroup\$ So, does my code represent the connected mode? another Q: I noticed that only EF Core edition has the disconnected mode based on this, am I true? \$\endgroup\$ – Ahmed Suror Feb 26 at 17:29
  • \$\begingroup\$ @AhmedSuror - if I understand correctly from provided link, connected mode means "tracked". Looking at your code, you can still short disposable contexts (connected mode). For each operation you can just create a new context, add new item / get entity to change, perform changes and SaveChanges. \$\endgroup\$ – Alexei Feb 26 at 20:01

Not the answer you're looking for? Browse other questions tagged or ask your own question.