4

i have a Function with a lot of parameters how can do like this print all parmetrs Kay, Value

public string GetCompatibility(int MediaId, int ProductsTypId, string id, string PreviousURL, HttpRequestBase Request, int? Width, int? Height, int? CampaignID)
{
      foreach (param in.GetEnvironmentVariables())
                 Console.WriteLine("  {0} = {1}", param.Key, param.Value);
}

I'm sorry I did not explain well enough the question

11
  • 11
    So many arguments is mostly a sign of bad design. Does your method really do exactly one thing? You should consider to split arguments into objects that belong together and devide your code into several sub-methods. Anyway you can use a set of arguments using the params-keyword: public string GetCompatibility(params object[] args). Commented Oct 10, 2016 at 14:41
  • That's completely impossible. Commented Oct 10, 2016 at 14:43
  • Maybe consider changing your set of arguments into properties of a class that is passed to the method? Commented Oct 10, 2016 at 14:43
  • I'm not professional enough, but I asked if it is possible Commented Oct 10, 2016 at 14:44
  • No. stackoverflow.com/questions/471693/… Commented Oct 10, 2016 at 14:45

2 Answers 2

0

Mostly having so many parameters on a method shows that your method does too many things. You should consider to refactor it into smaller chunks of logic.

Anyway if you think your method already is as small as possible you should put all parameters into a single DTO:

public class ParamValues
{
    public int MediaId {get; set; }
    // ...

    public ParamValues(int MediaId, int ProductsTypId, string id, string PreviousURL, HttpRequestBase Request, int? Width, int? Height, int? CampaignID)
    {
        this.MediaId = MediaId;
        // ...
    }
}

Now in your calling code:

GetCompatibility(new ParamValues(...));

Now you can easily log all your parameters, either using reflection on the ParamValues-class (bad idea) or directly within the constructor of ParamValues.

Another way is using the params-keyword:

public string GetCompatibility(params object[] args)
{
    foreach(var arg in args) Console.WriteLine(arg);
}

However if you use this approach you´ll use the arguments names. Moreover you simply hide the complexity of the method, which is a bad design, so you should chose that opportunity as last resort.

Sign up to request clarification or add additional context in comments.

4 Comments

I don't see how that code smells any better than the original. If he wants to iterate through the parameters to that method, he can copy and paste. If he'll be changing the parameter list so often that that's impractical, he's got much bigger problems than how to enumerate the parameters. If he wants to iterate through the parameters to a dozen different methods, defining a dozen parameter classes is plain nuts. Obviously, though, this is a solution to his problem. Or the problem he thinks he has, anyway.
@EdPlunkett You´re right, obviosly the main-problem of the OP is a bad design. In this case you´re right that this answer won´t solve that problem.
IMHO OP shouldn't even be told that params exists, he'll be using it everywhere and posting questions about why it says his int is a string or something.
@EdPlunkett Again you´re right, I added a comment for that.
0

You can use a Dictionary to arrange your paramters.

public static void GetCompatibility(Dictionary<string, object> paramters)
{
    foreach (var pair in paramters)
        Console.WriteLine("  {0} = {1}", pair.Key, pair.Value);
}

//...
Dictionary<string, object> paramters = new Dictionary<string, object>
{
    {"MediaId", 1},
    {"ProductsTypId", 1},
    {"PreviousURL", "http://xxx"},
    {"Request", null},
    {"Width", 123},
    {"Height", 123},
    {"CampaignID", 123},
};
GetCompatibility(paramters);
//...

Or, if the paramters are fixed, use a struct to do that.

public struct CompatibilityParam
{
    public int MediaId;
    public int ProductsTypId;
    public string id;
    public string PreviousURL;
    public HttpRequestBase Request;
    public int? Width;
    public int? Height;
    public int? CampaignID;
}
public static void GetCompatibility(CompatibilityParam paramters)
{
    Console.WriteLine("  MediaId = {0}", paramters.MediaId);
    Console.WriteLine("  ProductsTypId= {0}", paramters.ProductsTypId);
    Console.WriteLine("  PreviousURL= {0}", paramters.PreviousURL);
    Console.WriteLine("  Request= {0}", paramters.Request);
    Console.WriteLine("  Width= {0}", paramters.Width);
    Console.WriteLine("  Height= {0}", paramters.Height);
    Console.WriteLine("  CampaignID= {0}", paramters.CampaignID);
}

//...
CompatibilityParam paramters = new CompatibilityParam
{
    MediaId = 1,
    ProductsTypId = 1,
    PreviousURL = "http://xxx",
    Request = null,
    Width = 123,
    Height = 123,
};
GetCompatibility(paramters);
//...

2 Comments

This would, in my opinion, even be worse as your contract is now not clear anymore.
@L-Four It's not just your opinion.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.