patterncsharpMinor
combining 4 CRUD functions into 1?
Viewed 0 times
functionsintocombiningcrud
Problem
I have a console application that I'm trying to make with minimal redundant code.
I have 4 functions, Create, Read, Update, and Delete.
I also have a helper static class that provides some functions to make some tasks easier.
4 CRUD FUNCTIONS
```
class Program
{
#region CRUD functions
static private void DoCreate()
{
int myID;
var dbEntities = new TestDatabaseEntities();
string sNewName = "";
myID = stcHelper.InsistOnValidInput("Enter ID: ", "ID must be Valid: ", int.TryParse);
sNewName = stcHelper.GetInput("Enter Name", x => x.Trim());
stcHelper.TryDataBase(dbEntities, () =>
{
dbEntities.Men.Add(new Man{ManID = myID, Name = sNewName});
Console.WriteLine("Insert Sucessful");
});
stcHelper.SaveDBChanges(dbEntities);
}
static private void DoRead()
{
var dbEntities = new TestDatabaseEntities();
string sDisplayDivider = "|||||||||||||||||||||||||||||||||||||||";
var query = from person in dbEntities.Men
where true
select person;
Console.WriteLine(sDisplayDivider);
stcHelper.TryDataBase(dbEntities, () =>
{
foreach (Man m in query)
{
Console.WriteLine(m.ManID + " " + m.Name);
}
});
Console.WriteLine(sDisplayDivider);
}
static private void DoUpdate()
{
int myID;
var dbEntities = new TestDatabaseEntities();
string sNewName = "";
// get The Target's ID for the user to update
myID = stcHelper.GetInput("Enter ID to update: ", i
I have 4 functions, Create, Read, Update, and Delete.
- is it possible or practical to combine the four functions into one function that takes different parameters to create, read, update, or delete?
- is it possible to also adjust that one combined function to use whichever of my two entity tables (Man or Location) I tell it to use?
I also have a helper static class that provides some functions to make some tasks easier.
4 CRUD FUNCTIONS
```
class Program
{
#region CRUD functions
static private void DoCreate()
{
int myID;
var dbEntities = new TestDatabaseEntities();
string sNewName = "";
myID = stcHelper.InsistOnValidInput("Enter ID: ", "ID must be Valid: ", int.TryParse);
sNewName = stcHelper.GetInput("Enter Name", x => x.Trim());
stcHelper.TryDataBase(dbEntities, () =>
{
dbEntities.Men.Add(new Man{ManID = myID, Name = sNewName});
Console.WriteLine("Insert Sucessful");
});
stcHelper.SaveDBChanges(dbEntities);
}
static private void DoRead()
{
var dbEntities = new TestDatabaseEntities();
string sDisplayDivider = "|||||||||||||||||||||||||||||||||||||||";
var query = from person in dbEntities.Men
where true
select person;
Console.WriteLine(sDisplayDivider);
stcHelper.TryDataBase(dbEntities, () =>
{
foreach (Man m in query)
{
Console.WriteLine(m.ManID + " " + m.Name);
}
});
Console.WriteLine(sDisplayDivider);
}
static private void DoUpdate()
{
int myID;
var dbEntities = new TestDatabaseEntities();
string sNewName = "";
// get The Target's ID for the user to update
myID = stcHelper.GetInput("Enter ID to update: ", i
Solution
First off, thank you for the formatting and white space.
To answer your question, having the CRUD operations into one is not possible. Somewhere you have to have different logic to do the operations, which is the standard way of doing thing
Looking through your code, I see a few issues. Having your data access, U.I. and entities in one class violates the Single Responsibility Principle all over the place. Basically a class/method should do one thing, and do it well.
Your entities, data access and U.I. should be separated out into their own classes, maybe even their own libraries. When I read any class with the word 'helper' in it, I cringe, especially when its static. There is usually a better to write that code, and still follow SOLID.
I would change it something more like this:
This code will decouple your logic, so in the future if you wish to change how the data is stored, you just change the DataAccess code. If you want to make it a windows app, you replace the UserInterface class. In fact, I would create a interfaces and inject concrete classes where they are required. But this is a discussion for another time.
A couple more comments:
Just a quick note: this is done in notepad so the code might not be 100% accurate, but hopefully it portrays the general ideas.
Good luck.
To answer your question, having the CRUD operations into one is not possible. Somewhere you have to have different logic to do the operations, which is the standard way of doing thing
Looking through your code, I see a few issues. Having your data access, U.I. and entities in one class violates the Single Responsibility Principle all over the place. Basically a class/method should do one thing, and do it well.
Your entities, data access and U.I. should be separated out into their own classes, maybe even their own libraries. When I read any class with the word 'helper' in it, I cringe, especially when its static. There is usually a better to write that code, and still follow SOLID.
I would change it something more like this:
class DataAccess
{
public Man CreateMan(string name, ...)
{
// Logic required to create a man
}
public bool UpdateMan(Man man)
{
// Logic required to update
}
public bool DeleteMan(Man man)
{
// Logic required to delete
}
public Man GetMan(int id)
{
// Logic required to retrieve
}
}
public class UserInterface
{
public T GetInput(string prompt)
{
}
public T InsistOnValidInput(string prompt, TryParse validator)
{
}
}
public static class Program
{
private DataAccess _dataAccess;
private UserInterface _userInterface;
public int Main()
{
_dataAccess = new DataAccess();
_userInterface = new UserInterface();
var input = string.Empty;
while(input != "quit")
{
input = _userInterface.GetInput("Enter operation:");
ProcessInput(input);
}
}
public void ProcessInput(string input)
{
switch(input)
{
case "new Person":
CreatePerson();
break;
case "delete person":
DeletePerson()
break;
// other operations
}
}
private CreatePerson()
{
var firstName = _userInterface .InsistOnValidInput("Enter first name: ", i => i.Trim());
var age = _userInterface .InsistOnValidInput("Enter age: ", int.TryParse);
var person = _dataAcess.CreatePerson(firstName, age, ...);
}
// More operations
}This code will decouple your logic, so in the future if you wish to change how the data is stored, you just change the DataAccess code. If you want to make it a windows app, you replace the UserInterface class. In fact, I would create a interfaces and inject concrete classes where they are required. But this is a discussion for another time.
A couple more comments:
- Lose the regions. Regions just add unnecessary and confusing code. If you need to wrap code in a region, there is a good chance that code can be pulled out into its own class.
- Most of the comments add nothing to the code. // Execute the query, and change the column values is fairly obvious to anybody that can read that code. Comments should be used sparingly to justify why you did something, explain the reasoning behind what looks like complex code, or to clarify something. i.e. // Bug in X library so this has to be done first
- Most of the places you've created static methods, static is being used incorrectly. You have basically create a procedural application, rather than an object oriented application.
Just a quick note: this is done in notepad so the code might not be 100% accurate, but hopefully it portrays the general ideas.
Good luck.
Code Snippets
class DataAccess
{
public Man CreateMan(string name, ...)
{
// Logic required to create a man
}
public bool UpdateMan(Man man)
{
// Logic required to update
}
public bool DeleteMan(Man man)
{
// Logic required to delete
}
public Man GetMan(int id)
{
// Logic required to retrieve
}
}
public class UserInterface
{
public T GetInput<T>(string prompt)
{
}
public T InsistOnValidInput<T>(string prompt, TryParse<T> validator)
{
}
}
public static class Program
{
private DataAccess _dataAccess;
private UserInterface _userInterface;
public int Main()
{
_dataAccess = new DataAccess();
_userInterface = new UserInterface();
var input = string.Empty;
while(input != "quit")
{
input = _userInterface.GetInput<string>("Enter operation:");
ProcessInput(input);
}
}
public void ProcessInput(string input)
{
switch(input)
{
case "new Person":
CreatePerson();
break;
case "delete person":
DeletePerson()
break;
// other operations
}
}
private CreatePerson()
{
var firstName = _userInterface .InsistOnValidInput<string>("Enter first name: ", i => i.Trim());
var age = _userInterface .InsistOnValidInput<int>("Enter age: ", int.TryParse);
var person = _dataAcess.CreatePerson(firstName, age, ...);
}
// More operations
}Context
StackExchange Code Review Q#18197, answer score: 7
Revisions (0)
No revisions yet.