HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpModerate

How can I make this StringToCollections method more generic?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisgenericcanmethodmakemorestringtocollectionshow

Problem

I am new to C# and I have big functions that receives the following arguments:

(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 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.