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:
- 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.
- 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.
catch{}
is a terrible idea. You should only catch exceptions that you know about. \$\endgroup\$catch
is just hiding a bug. And most of the time, operation shouldn't silently fail. If I callInsertUpdateRecords()
and it returns successfully, I will expect that it succeeded. You should only catch an exception is you know what to do with it. \$\endgroup\$int flag
is a bad data type to use and a bad parameter name. Consider creating anenum
called something likeOperation
withInsert
andUpdate
as enumeration sub-values. Make that the parameter type to your method. \$\endgroup\$