snippetcsharpModerate
How can I make this StringToCollections method more generic?
Viewed 0 times
thisgenericcanmethodmakemorestringtocollectionshow
Problem
I am new to C# and I have big functions that receives the following arguments:
Asides from the first argument, which is an object of type
So, for example, a valid input for this function would be:
A package is an object that contains a list of materials, a list of functions and a list of companies.
The objective of this function, is to look up in the DB all the objects that contain those IDs, and add them to
To achieve this objective, this is what I have created:
```
private Package StringToCollections(Package inPackage, string materialIDs, string functionIDs, string companyIDs)
{
//Check Materials Data - Case WITHOUT FORMAT PROTECTION
var materialList = new List();
var materialStringArray = materialIDs.Split(',');
var materialIdsArray = materialStringArray.Where(x => x != "").Select(int.Parse);
materialList.AddRange(DB.Materials.Where(x => materialIdsArray.Contains(x.MaterialId)));
inPackage.GATE_Material = materialList;
//Check Employee_Functions Data - HAS FORMAT PROTECTION ... KINDOFF?
List functionsList = new List();
string[] functionStringArray = functionIDs.Split(',');
List functionIdsArray = new List();
for (int i = 0; i functionIdsArray.Contains(x.FunctionId)));
inPackage.Function = functionsList;
//Check Company Data - ANOTHER WAY OF FORMAT PROTECTION ... KINDOFF?
if (!companyIDs.IsNullOrWhiteSpace())
{
var companiesList = new List();
var companyStringArray = companyIDs.Split(',');
var companyIdsArray = companyStringArray.Where(x => x != "").Select(int.Parse);
try
{
comp
(Package inPackage, string materialIDs, string functionIDs, string companyIDs)Asides from the first argument, which is an object of type
Package, all the other arguments are strings of IDs. So, for example, a valid input for this function would be:
myPackage, "1,2,3", ",2,3,4", ",45,2".A package is an object that contains a list of materials, a list of functions and a list of companies.
The objective of this function, is to look up in the DB all the objects that contain those IDs, and add them to
myPackage.To achieve this objective, this is what I have created:
```
private Package StringToCollections(Package inPackage, string materialIDs, string functionIDs, string companyIDs)
{
//Check Materials Data - Case WITHOUT FORMAT PROTECTION
var materialList = new List();
var materialStringArray = materialIDs.Split(',');
var materialIdsArray = materialStringArray.Where(x => x != "").Select(int.Parse);
materialList.AddRange(DB.Materials.Where(x => materialIdsArray.Contains(x.MaterialId)));
inPackage.GATE_Material = materialList;
//Check Employee_Functions Data - HAS FORMAT PROTECTION ... KINDOFF?
List functionsList = new List();
string[] functionStringArray = functionIDs.Split(',');
List functionIdsArray = new List();
for (int i = 0; i functionIdsArray.Contains(x.FunctionId)));
inPackage.Function = functionsList;
//Check Company Data - ANOTHER WAY OF FORMAT PROTECTION ... KINDOFF?
if (!companyIDs.IsNullOrWhiteSpace())
{
var companiesList = new List();
var companyStringArray = companyIDs.Split(',');
var companyIdsArray = companyStringArray.Where(x => x != "").Select(int.Parse);
try
{
comp
Solution
Consistency:
Don't mixup using
Combining methods:
Use method chaining to shorten your code. There's no need to create a variable for every step your code does. This:
can be changed to:
Generics:
You cannot make one magic generic method to do this. Why? Simply because you do not know the type ahead of time. What you can do however is to create separate method for getting the valid ids from the string:
Now your code looks much cleaner and is easier to read and maintain, not only by you but also other people.
You can also assign the database call directly to the according property of
Don't mixup using
var and declaring your variables explicitly. Choose one of the two, preferably var.Combining methods:
Use method chaining to shorten your code. There's no need to create a variable for every step your code does. This:
var materialStringArray = materialIDs.Split(',');
var materialIdsArray = materialStringArray.Where(x => x != "").Select(int.Parse);can be changed to:
var materialIdsArray = materialIDs.Split(',').Where(x => x != "").Select(int.Parse);Generics:
You cannot make one magic generic method to do this. Why? Simply because you do not know the type ahead of time. What you can do however is to create separate method for getting the valid ids from the string:
public IEnumerable GetValidIDs(string ids)
{
foreach (var id in ids.Split(','))
{
int x;
if (Int32.TryParse(id, out x))
{
yield return x;
}
}
}
private Package StringToCollections(Package inPackage, string materialIDs, string functionIDs, string companyIDs)
{
var validMaterialIDs = GetValidIDs(materialIDs);
var materials = DB.Materials.Where(x => validMaterialIDs.Contains(x.MaterialId));
var validFunctionIDs = GetValidIDs(functionIDs);
var functions = DB.Function.Where(x => validFunctionIDs.Contains(x.FunctionId)));
var validCompanyIDs = GetValidIDs(functionIDs);
var companies = DB.Companies.Where(x => validCompanyIDs.Contains(x.FunctionId)));
inPackage.GATE_material = materials;
inPackage.Function = functions;
inPackage.Companies = companies;
return inPackage;
}Now your code looks much cleaner and is easier to read and maintain, not only by you but also other people.
You can also assign the database call directly to the according property of
inPackage, for example:var validMaterialIDs = GetValidIDs(materialIDs);
inPackage.GATE_material = DB.Materials.Where(x => validMaterialIDs.Contains(x.MaterialId));Code Snippets
var materialStringArray = materialIDs.Split(',');
var materialIdsArray = materialStringArray.Where(x => x != "").Select(int.Parse);var materialIdsArray = materialIDs.Split(',').Where(x => x != "").Select(int.Parse);public IEnumerable<int> GetValidIDs(string ids)
{
foreach (var id in ids.Split(','))
{
int x;
if (Int32.TryParse(id, out x))
{
yield return x;
}
}
}
private Package StringToCollections(Package inPackage, string materialIDs, string functionIDs, string companyIDs)
{
var validMaterialIDs = GetValidIDs(materialIDs);
var materials = DB.Materials.Where(x => validMaterialIDs.Contains(x.MaterialId));
var validFunctionIDs = GetValidIDs(functionIDs);
var functions = DB.Function.Where(x => validFunctionIDs.Contains(x.FunctionId)));
var validCompanyIDs = GetValidIDs(functionIDs);
var companies = DB.Companies.Where(x => validCompanyIDs.Contains(x.FunctionId)));
inPackage.GATE_material = materials;
inPackage.Function = functions;
inPackage.Companies = companies;
return inPackage;
}var validMaterialIDs = GetValidIDs(materialIDs);
inPackage.GATE_material = DB.Materials.Where(x => validMaterialIDs.Contains(x.MaterialId));Context
StackExchange Code Review Q#71484, answer score: 11
Revisions (0)
No revisions yet.