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

Return a List<MyObject> based on very similar data

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

Problem

I am rewriting an old VB.NET app in C#. I will not subject you to the old VB.NET code that I am writing (the original method that I have already refactored was over 700 lines long).

The following is what we call On Demand Reports. There is a stored procedure that these objects fill in for us. I don't have a 100% understanding of why most of what I am doing is just static string text but it is.

What I want to do is to find a better way to represent the List (see the embedded comments).

```
private IEnumerable GetOnDemandInputsBy(int key)
{
var inputs = new List();
//This is actually called 13 times with different parameter variables (the parameter called parameter) and a different static dictionary
inputs.Add(AssembleMyObject(key, "@BeginTime", MyObjectPropertyConstants.BeginTime()));
inputs.Add(AssembleMyObject(key, "@EndTime", MyObjectPropertyConstants.EndTime()));

return inputs;
}

//This method is fine with me. It is just here for context
private static IEnumerable AssembleMyObjectProperty(Dictionary dictionary)
{

return dictionary.Select(item => new MyObjectProperty {PropertyName = item.Key, PropertyValue = item.Value});
}

//I feel like I am passing to many things into this method and its doing to much
private MyObject AssembleMyObject(int key, string parameter, Dictionary dictionary)
{
//this is set to 0 in the constructor, I am not sure why the original developers chose 10 for every new object
_displayOrder += 10;
var MyObject = new MyObject
{
DisplayOrder = _displayOrder,
ParameterName = parameter,
QueryKey = key,
};
MyObject.MyObjectProperties.AddRange(AssembleMyObjectProperty(dictionary));
return MyObject;
}

//This seems like a bad idea as well but all the data is

Solution

var inputs = new List();
inputs.Add(AssembleMyObject(key, "@BeginTime", MyObjectPropertyConstants.BeginTime()));
inputs.Add(AssembleMyObject(key, "@EndTime", MyObjectPropertyConstants.EndTime()));

return inputs;


You could rewrite this to make it more DRY using LINQ into:

return new Func>[]
{
    MyObjectPropertyConstants.BeginTime,
    MyObjectPropertyConstants.EndTime
}.Select(f => AssembleMyObject(key, "@" + f.Name, f()).ToList();


var MyObject


Local variables are usually named in camelCase, e.g. myObject.

public static Dictionary BeginTime()


Method should be named using verbs, this looks more like a property. Thought making this into a property would make the above LINQ rewrite more difficult.

new Dictionary
{
    {"stuff", "@BeginTime"},
    {"VISIBLE", "TRUE"},
    {"CAPTION", "*beginTime"},
    {"DISPLAYTYPE", "DATEPICKER"},
    {"DATATYPE", "DATE"},
};


Does order matter here? If it does, you shouldn't use Dictionary, since it doesn't guarantee ordering.

Code Snippets

var inputs = new List<MyObject>();
inputs.Add(AssembleMyObject(key, "@BeginTime", MyObjectPropertyConstants.BeginTime()));
inputs.Add(AssembleMyObject(key, "@EndTime", MyObjectPropertyConstants.EndTime()));

return inputs;
return new Func<Dictionary<string, string>>[]
{
    MyObjectPropertyConstants.BeginTime,
    MyObjectPropertyConstants.EndTime
}.Select(f => AssembleMyObject(key, "@" + f.Name, f()).ToList();
var MyObject
public static Dictionary<string, string> BeginTime()
new Dictionary<string, string>
{
    {"stuff", "@BeginTime"},
    {"VISIBLE", "TRUE"},
    {"CAPTION", "*beginTime"},
    {"DISPLAYTYPE", "DATEPICKER"},
    {"DATATYPE", "DATE"},
};

Context

StackExchange Code Review Q#57827, answer score: 2

Revisions (0)

No revisions yet.