4

I am writing a program that needs to be able to process data from a number of different sources. The sources output data in a variety of formats. So, depending on which source is being used, I need to initialize a string list with a different set of column headers. The quantity of headers varies between 10 and 18 columns depending on the data source.

Essentially, I need a clean method to conditionally initialize a list. I have three working solutions but I am not sure which is the cleanest/best practice:

  • Example #1 requires that I add each header to the list line-by-line. With the number of potential sources and columns that need to be added for each, this list gets pretty long.
  • Example #2 requires that I use {} around each case statement to limit the scope so that I can declare the dataHeaders list individually in each case statement. I have never encountered a situation wherein I had to limit the scope to an individual case statement and as such, I suspect it is bad practice.
  • Example #3 seems like the best option. I actually implemented this one as I was writing this question. I don't really see any drawbacks here but I thought I'd still pose the question anyhow.

Is there a better/cleaner way to do this? If not, which example is the best practice?

Example #1:

List<string> headers = new List<string>();
switch (dataSource)
{
    case "Source 1":
        headers.Add("header1");
        headers.Add("header2");
        headers.Add("header3");
        headers.Add("...");
        headers.Add("header18");
        break;
    case "Source 2":
        headers.Add("headerA");
        headers.Add("headerB");
        headers.Add("headerC");
        headers.Add("...");
        headers.Add("headerJ");
        break;
    default:
        // TODO: Handle bad sources.
        break;
}

Example #2:

List<string> headers = new List<string>();
switch (dataSource)
{
    case "Source 1":
        {
            List<string> dataHeaders = new List<string>() { "header1", "header2", "header3", "...", "header18" }
            headers = dataHeaders;
            break;
        }
    case "Source 2":
        {
            List<string> dataHeaders = new List<string>() { "headerA", "headerB", "headerC", "...", "headerJ" }
            headers = dataHeaders;
            break;
        }
    default:
        {
            // TODO: Handle bad sources.
            break;
        }
}

Example #3:

List<string> headers = new List<string>();
switch (dataSource)
{
    case "Source 1":
        headers.AddRange(new List<string>() { "header1", "header2", "header3", "...", "header18" })
        break;
    case "Source 2":
        headers.AddRange(new List<string>() { "headerA", "headerB", "headerC", "...", "headerJ" })
        break;
    default:
        // TODO: Handle bad sources.
        break;
}
  • Option number two does not have the same effect as the others. It is replacing the header list created in the outer scope with the one created in the inner scope one, whereas the others (1&3) are instead extending the list declared in the outer scope. Otherwise, there is not enough information to give a thoughtful answer. For example, can any of these rather static lists be reused? How often would these snippets be invoked: just one-time for configuration, or frequently? Why are we using so many strings instead of more specific data types? – Erik Eidt May 27 '16 at 0:41
  • I would create both the headers1... list and headersA... list and in the case statement insert one of those. – Pieter B May 27 '16 at 7:22
3

A lot of times, you can really clean up a problem like this by expressing your data in a form with less syntax than a programming language, such as a CSV string. I don't know C#, but an example in Scala is:

def headersForDataSource(dataSource: String): Option[Array[String]] = {
  val headers = """
    Source1,header1,header2,header3,...,header18
    Source2,headerA,headerB,headerC,...,headerJ
    """

  val lines = headers.lines map (_.trim split ',')
  lines find (_(0) == dataSource) map (_.tail)
}

You can see this makes your list of headers significantly more compact and easier to read, because it doesn't clutter and intermingle it with the syntax of the data structure. It's putting it in your preferred format, instead of the format forced on you by the programming language.

|improve this answer|||||
1

What is the issue you're trying to fix?

If you're looking to improve performance, then this is definitely a case of premature optimisation. Any of the three methods you give could fill a list with thousands of items in the fraction of a second it takes to lift your finger from the mouse after clicking on a button.

If you're just trying to make your code less verbose, then a variation on option 3 is the best.

var headers = new List<string>();
switch (dataSource)
{
    case "Source 1":
        headers.AddRange(new [] { "header1", "header2", "header3", "...", "header18" })
        break;
    case "Source 2":
        headers.AddRange(new [] { "headerA", "headerB", "headerC", "...", "headerJ" })
        break;
    default:
        // TODO: Handle bad sources.
        break;
}

Note that you don't have to specify the type of the list, or the arrays for the elements - these will be automatically inferred by the compiler.

|improve this answer|||||
1

You also can avoid switch and if using maps. I'm going to use Java syntaxis, but I guess it could be done in C# easily

Unmodificable (static or constant) Descriptor

Map<String,String[]> HEADERS_MAP = new HashMap<String,String[]>();
HEADERS_MAP.put("Source 1",new String[]{"header1","headerN"});
HEADERS_MAP.put("Source 2",new String[]{"headerA","headerZ"});
...
//Preventing Map from changes
HEADERS_MAP = Collections.unmodifiableMap(HEADERS_MAP);

Note: I think Maps are Dictorinary in C#

Header builder

//Each consumer will have it's onw list
return Arrays.asList(HEADERS_MAP.get(dataSource));

Edit: I have edited by first answer @unholysampler's comment.

Now Map stores String[] and builder returns a new fresh/unbound list. So builder consumer will be able to modify the content of the list and it will prevent code from unwanted changes. It could be done with Collections.unmodificableListbut that way adds little bit more code and complexity. It does code less readable.

|improve this answer|||||
  • 1
    If you are going to use this style, you should be aware that each caller is getting a reference to the same list instance. If the first caller mutates the list, all other callers will see the change. That isn't to say you should never do it, but it is a hard issue to track down when you are not aware of this potential issue. – unholysampler May 27 '16 at 13:44
  • @unholysampler True! I went stright to the easy solution, because I don't know if there're restrictions. However, I think it's a good point to prevent the solution from changes. I have edited the answer adding a little control to prevent Map and List from changes. Hope it helps – Laiv May 27 '16 at 14:11
0

I think a slightly modified option 2 is your best bet. Leave the declaration of headers outside the switch, and inside the switch just set headers = new List

You also don't need the curly braces to create blocks for each case statement; C# will force you to have break at the end of each case.

The advantage of doing this is that if you add a new case, you'll be warned that headers isn't initialized if you try to use it later, where as your current #2 will just end up with an empty list, which may be a logic error to track down later.

|improve this answer|||||
0

Here is shorter version, combining suggestions of Peregrine and Karl Bielefeldt:

string hline;
switch (dataSource)
{
    case "Source 1":
        hline = "header1,header2,header3,...,header18";
        break;
    case "Source 2":
        hline = "headerA,headerB,headerC,...,headerZ";
        break;
    default:
        // TODO: Handle bad sources.
        break;
}
var headers = new List<string>();
headers.AddRange(hline.Split(','));

If you can refer to your different data sources by an index from a consecutive interval, I would consider to avoid the whole switch-case at all, and initialize hline like this:

 string hLine = new []{
   "header1,header2,header3,...,header18",
   "headerA,headerB,headerC,...,headerZ",
   "..."}[dataSourceIndex];
|improve this answer|||||
0

Why not use a Dictionary?

var headers = new List<string>();
var headersToAdd = new Dictionary<string, string[]>
{
    {"Source 1", new string[] {"header1", "header2"} },
    {"Source 2", new string[] {"headerA", "headerB"} }
};

headers.AddRange(headersToAdd["Source 1"]);
headers.AddRange(headersToAdd["Source 2"]);
|improve this answer|||||
  • if using dictionary why not store list as a value and skip one additional function call? – kuskmen Sep 8 '17 at 13:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.