Inserting and updating record using LINQ by calling a common function

Question

I have a function named InserUpdateRecords(int flag) to which I pass integer variable which indicates the type of operation it has to perform , if flag==1 then insert , if flag==2 then update. Here is my code:

public void InsertUpdateRecords(int flag)
{
 Data_Customer_Log customer_log=new Data_Customer_Log();
 try
 {
 if(flag==2)
 {
 customer_log = (from cust in dt.Data_Customer_Logs
 where cust.cApplicationNo == Request.QueryString["ApplicationNO"]
 select cust).SingleOrDefault(); 
 }
 customer_log.cCustomerType = NewCustomerddlCustomerType.Text;
 customer_log.cBranchName = NewCustomerddlBranchName.Text;
 customer_log.nBranchNo = Convert.ToDecimal(Global.gBranchNo);
 .
 .
 .
 .
 }
 catch{}
}

So when I create Data_Customer_Log customer_log=new Data_Customer_Log(); and then depending on the value of flag I use it , is it an appropriate approach, what are the possible downsides to my approach.

• svick: Using catch{} is a terrible idea. You should only catch exceptions that you know about.
• svick: Like I said, you should only catch specific exceptions that you know about. If an unknown exception is thrown, your catch is just hiding a bug. And most of the time, operation shouldn't silently fail. If I call InsertUpdateRecords() and it returns successfully, I will expect that it succeeded. You should only catch an exception is you know what to do with it.
• Jesse C. Slicer: int flag is a bad data type to use and a bad parameter name. Consider creating an enum called something like Operation with Insert and Update as enumeration sub-values. Make that the parameter type to your method.

Answer 1

I would probably pass in the customer log as a parameter to the method and then have it deal with just the database insertion. If you wanted to keep a InsertOrUpdate type of approach I would branch on perhaps something like the object ID if it has one. This way you can have a variety of ways that your object is created and the method is only responsible for the persistance of that object not also it's creation and populating.

A couple of problems I see in your approach:

1. The method is doing too many things. It's responsible for querying for your record, instantiating the record and populating as well as performing the persistance.
2. The method has too many dependencies. It looks like you have made it subject to both an external UI component in the .Text objects as well as the database layer. What happens if you had multiple ways of populating the object. You would have to duplicate this method?

Something like this (without knowing a bit more of your setup):

// a method higher up the chain
void CreateCustomerLog()
{
 Data_Customer_Log record = new Data_Customer_Log();
 record.cCustomerType = NewCustomerddlCustomerType.Text;
 record.cBranchName = NewCustomerddlBranchName.Text;
 record.nBranchNo = Convert.ToDecimal(Global.gBranchNo);
 InsertOrUpdate(record);
}
void UpdateCustomerLog(string applicationNo)
{
 Data_Customer_Log record = GetCustomerLog(applicationNo);
 if(record == null)
 {
 // handle record not existing here or filter it up the chain
 }
 else
 {
 InsertOrUpdate(record);
 }
}
Data_Customer_Log GetCustomerLog(string applicationNo)
{
 return (from cust in
 dt.Data_Customer_Logs
 where cust.cApplicationNo.Equals(applicationNo)
 select cust).SingleOrDefault(); 
}
Data_Customer_Log InsertOrUpdate(Data_Customer_Log record)
{
 if(record.ID > 0)
 { 
 return Insert(record);
 }
 else 
 {
 return Update(record);
 }
}
Data_Customer_Log Insert(Data_Customer_Log record)
{
 // TODO: perform insertion and throw exception if there is a problem
}
Data_Customer_Log Update(Data_Customer_Log record)
{
 // TODO: perform update and throw exception if there is a problem
}

I would probably point out that these methods would probably be in different classes. For example the Update, Insert and/or Get might be in some sort of repository class. The CreateCustomerLog and UpdateCustomerLog might be at the higher level, maybe your UI level as it interacts directly with the UI elements.

Just my thoughts.

• Priyank Patel: thanks a lot , you mean to say that I should insert and update records in different function and only pass the type of object which would let it know if it is insert or update , I could no understand your use of InserOrUpdate() , could you explain.
• Priyank Patel: in Insert and Update methods should I only do dt.insertOnSubmit() and dt.SubmitChanges() respectively .
• dreza: @freebird yes, that was what i was trying to get at

Answer 2

// Add/Update or CustomerLog for the given Application number.
public bool StoreCustomerLog(string applicationNO)
{
 bool result = false;
 using(CustomerEntities context = new CustomerEntities())
 {
 // See if our record is already there.
 Data_Customer_Log record = (from row in context.Data_Customer_Logs
 where row.cApplicationNo == applicationNO
 select row).FirstOrDefault();
 // If we didn't get a record back, it's an insert
 bool isNew = (record == null);
 if(isNew)
 {
 // Create an instance of the entity
 record = context.Data_Customer.CreateObject();
 // Add any other fields that are created on insert only.
 }
 // Fields that are set on insert and updates ...
 // -- FYI: I wouldn't do this directly, but pass these values
 // ------- as parameters so they could be validated prior to 
 // ------- storing in the database to avoid insert/update exceptions.
 record.cCustomerType = NewCustomerddlCustomerType.Text;
 record.cBranchName = NewCustomerddlBranchName.Text;
 // Other fields ...
 // If it's new, we add it to the entity collection
 if(isNew) { context.Data_Customer.AddObject(record); }
 // If we affected a row, then we were successful!
 result = (context.SaveChanges() > 0);
 }
 return result;
}

That's how I'd do it.

• Priyank Patel: thanks for the idea , but I am no very clear how would I go about this , may be I have not posted the entire code , should I post full code so that you could help me better.
• Paul Duncan: Yep - that would help.