Return custom assembly attribute variables

Question

I have a custom attribute in my assembly called SemverAttribute, and I have a helper class called AppInfo that has a function to return a number called the Semver number. It accepts an id of null-6. Is this the most efficient way to do this or should I break all this up into separate strings instead of one inline function with an argument?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Reflection;
using authenticator.Properties;
namespace authenticator.Helpers
{
 public class AppInfo
 {
 // Grab the SemVer from assembly
 // You may pass any value 0-6 or null for id
 // null or 0 will return the entire SemVer string (X.Y.Z-pre+meta)
 // 1 will return the SemVer Major.Minor.Patch/Micro (X.Y.Z)
 // 2 will return the SemVer Major (X)
 // 3 will return the SemVer Minor (Y)
 // 4 will return the SemVer Patch/Micro (Z)
 // 5 will return the SemVer Pre data
 // 6 will return the SemVer Meta data
 public static string SemverPart(int id = 0)
 {
 var attribute = Assembly.GetExecutingAssembly().GetCustomAttributes(false).OfType<SemverAttribute>().FirstOrDefault();
 string str = (attribute == null) ? string.Empty : attribute.getversion;
 // Define delimiter to split pre, and meta information off the string
 string[] d = { "-", "+" };
 // Convert SemVer string to array
 var n = str.Split(d, StringSplitOptions.RemoveEmptyEntries);
 if (id > 0 & id <= 6 )
 {
 if (id >= 1 & id < 5)
 {
 // Grab the full version number
 string full = n[0];
 // Define delimiter to split version number into Major.Minor.Patch/Micro
 string[] x = { "." };
 // Convert version string to array
 var v = full.Split(x, StringSplitOptions.RemoveEmptyEntries);
 // Passing 1 returns entire version number (X.Y.Z)
 if (id == 1)
 {
 return (full == null) ? string.Empty : full;
 }
 // Passing 2 returns major version number (X)
 else if (id == 2)
 {
 return (v[0] == null) ? string.Empty : v[0];
 }
 // Passing 3 returns minor version number (Y)
 else if (id == 3)
 {
 return (v[1] == null) ? string.Empty : v[1];
 }
 // Passing 4 returns patch/micro version number (Z)
 else
 {
 return (v[2] == null) ? string.Empty : v[2];
 }
 }
 // Passing 5 returns pre data
 else if (id == 5)
 {
 return (n[1] == null) ? string.Empty : n[1];
 }
 // Passing 6 returns the meta data
 else
 {
 return (n[2] == null) ? string.Empty : n[2];
 }
 } 
 // Return full string if no argument is passed or an invalid argument is passed
 else 
 {
 return str;
 }
 }
 }
}

EDIT: Version 3 based on answer by unholysamper. This can still be improved if I can break up the version string ONLY if 1-4 or selected.:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Reflection;
using Authenticator.Properties;
namespace Authenticator.Helpers
{
 public class AppInfo
 {
 // Grab the SemVer from assembly
 // You may pass any value 0-6 for id
 // 0 or nothing will return the entire SemVer string (X.Y.Z-pre+meta)
 // 1 will return the SemVer Major.Minor.Patch/Micro (X.Y.Z)
 // 2 will return the SemVer Major (X)
 // 3 will return the SemVer Minor (Y)
 // 4 will return the SemVer Patch/Micro (Z)
 // 5 will return the SemVer Pre data
 // 6 will return the SemVer Meta data
 public static string SemverPart(int id = 0)
 {
 var attribute = Assembly.GetExecutingAssembly().GetCustomAttributes(false).OfType<SemverAttribute>().FirstOrDefault();
 string SemVer = (attribute == null) ? string.Empty : attribute.getversion;
 string[] delimiter = { "-", "+" };
 var SemVerArray = SemVer.Split(delimiter, StringSplitOptions.RemoveEmptyEntries);
 var VersionArray = SemVerArray[0].Split('.');
 switch (id)
 {
 case 1:
 return SemVerArray[0] ?? string.Empty;
 case 2:
 return VersionArray[0] ?? string.Empty;
 case 3:
 return VersionArray[1] ?? string.Empty;
 case 4:
 return VersionArray[2] ?? string.Empty;
 // Determine if SemVer contains pre data and return it if it does
 case 5:
 if (SemVer.Contains("-"))
 {
 return SemVerArray[1] ?? string.Empty;
 }
 else 
 {
 return string.Empty;
 }
 case 6:
 if (SemVer.Contains("-"))
 {
 return SemVerArray[2] ?? string.Empty;
 }
 else
 {
 return SemVerArray[1] ?? string.Empty;
 }
 default:
 return SemVer;
 }
 }
 }
}

• Jeroen Vannevel: Why are you using dynamic instead of var or string[]?
• Raphaël Althaus: just use var, which will represent a string[]. dynamic should be used only when it's really necessary (so in many many case : never).
• Magus: Dynamic basically means the compiler doesn't worry about the type (which is generally a bad thing, but sometimes needed). var accepts the type that is assigned to it - in this case string[]
• Magus: In C#, public things are PascalCase. Private and member variables are camelCase. Some people make member variables _hungarianCamelCase, but your IDE can color them differently anyway, so the advantage of doing so is very much debatable. For the most part, snake_case isn't a thing in C#.
• Magus: The best place is probably this MSDN page. I'd also like to mention that your current code does not, in fact, support nulls. You could change the argument to an int? to allow that, but only if you really need to.

Answer 1

You have a lot of comments saying what you are doing (almost one for every line of code). Code is good at saying what is happing, comments are better at saying why something is happening. If it is not clear to a reader what the code does, this is an indication that you might need better variable names or more descriptive method names. Breaking a block of coding into a separate private function is a good way to make the code more readable.

---

(n[1] == null) ? string.Empty : n[1];

is equivalent to

n[1] ?? string.Empty;

---

if (id == 1) {
 return 1;
}
else if (id == 2)
{
 return 2;
} //...

is equivalent to

switch (id) {
case 1:
 return 1;
case 2:
 return 2;
//...
}

• aaronmallen: See edit version 2. The problem now is I'm being told not all code paths return a value.
• unholysampler: @aaronmallen: This is likely due to the compiler not being smart enough to know that the if clauses excluding all other values for id. You can add a default case that throws an exception saying "this should never happen".
• aaronmallen: See edit version 3: Still one improvement that can be made here. If I can I initiate breaking up the Major.Minor.Patch ONLY when option 2-4 are selected? I tried wrapping those variables in an if statement first but I couldn't figure out how to call them since wrapped in an if statement they wouldn't be in the context and I don't know how to define var VersionArray without assigning it a value first which can't be assigned a value without first assigning string version a value.

Answer 2

This would be a lot cleaner as a switch

 // Passing 1 returns entire version number (X.Y.Z)
if (id == 1)
{
 return (full == null) ? string.Empty : full;
}
// Passing 2 returns major version number (X)
else if (id == 2)
{
 return (v[0] == null) ? string.Empty : v[0];
}
//etc...

Compared to:

 // Passing 1 returns entire version number (X.Y.Z)
switch(id)
{
 case 1:
 return (full == null) ? string.Empty : full;
 case 2:
 return (v[0] == null) ? string.Empty : v[0];
 //etc...
}

Note that if you use a switch without a return, you would need to break;, as the code would continue to "fall through".