Is there a better way to handle passing in multiple parameters to methods

Question

I find myself doing the following a lot:

/**
 * Redirect to a MVC controller&action
 * @param controller
 * @param action
 */
public void redirect(String controller, String action) {
    redirect(controller, action, new HashMap<String, String>());
}
/**
 * Redirect to a MVC controller&action with extra URL parameters
 * @param controller
 * @param action
 * @param data
 */
public void redirect(String controller, String action, Map<String, String> data) {
    String urlParameters = "";
    for(String key : data.keySet()) {
        urlParameters += "&" + key + "=" + data.get(key);
    }
    m_binder.putLocal("RedirectParams", "IdcService=MVC_FRONTCONTROLLER&controller="+controller+"&action="+action + urlParameters);
}

To call the second method I actually need to create a hashmap to add data in it and I was wondering if there is a more convenient way of achieving this?

As you can see I need to know both the key and the value, so varargs wouldn't work (as far as I can see).

I'm open to all ideas, including using reflection.

I don't think writing 3 lines of extra code is so extreme... — Lukas Eder, Apr 26 '11 at 14:43
Switch to C# 4.0 or JavaScript. They have a very handy notation for exactly this pattern... — Codo, Apr 26 '11 at 14:45
@Lukas Eder -- it's not about the 3 extra lines of code, think about the iterations executed on each call and also the memory consumption (by the way you are using String concatenation thus creating a lot of temporary string which you are throwing away -- think of using a StringBuilder instead?) — Liv, Apr 26 '11 at 14:51
@Codo, great, hah I'll just simply turn over the "language switch" in my huge application. Why didn't I think of that earlier ;-) — Lukas Eder, Apr 26 '11 at 15:43
@Liv, then make it `null` by default and add a null check, then you can avoid the extra memory. Seriously, if you didn't measure it, then why worry about that little extra CPU/memory effort nowadays? — Lukas Eder, Apr 26 '11 at 15:44
I think not worrying about CPU/memory is not acceptable for anyone who had to deal with high throughput systems: simply creating an extra String to be passed to a logging framework and then gets gc'd is atrocious for a piece of code that gets called 1000's of times a minute -- you will experience JVM hangs as the GC kicks in. Though I'll give you the fact that no one mentioned high throughput here -- however one should always avoid taking the lazy approach and not consider these factors in. — Liv, Apr 26 '11 at 15:49
@Liv, thanks ;-) "redirect" is probably done once per request but never mind. — Lukas Eder, Apr 27 '11 at 07:18

score 4 · Answer 1 · answered Apr 26 '11 at 14:49

4

Why do you have to create a new map? I imagine you could just pass in null and then check for a null map in your second redirect method. It's probably a good idea to check for null anyway.

answered Apr 26 '11 at 14:49

Jeremy

22,188
4
68
81

score 4 · Answer 2 · answered Apr 26 '11 at 14:55

4

If the problem is that it is awkward to create a new map, for small maps you can use the guava ImmutbleMap.of() methods,

ImmutableMap.of("key1", "value1", "key2", "value2");

answered Apr 26 '11 at 14:55

sbridges

24,960
4
64
71

score 3 · Accepted Answer · answered Apr 26 '11 at 14:56

I wrote this convenience method for build maps a while back. It takes varargs and makes a map out of them in pairs. For creating simple maps in test code, this is quite convenient. You need to make sure you get the numbers of parameters right but i like it because it makes the code volume smaller.

@SuppressWarnings("unchecked")
public static <K, V> Map<K, V> mapOf(K key, V value, Object... morePairs) {
    Map<K, V> map = new HashMap<K, V>();
    map.put(key, value);
    for (int i=0; i<morePairs.length; i+=2) {
        map.put((K)morePairs[i], (V)morePairs[i+1]);
    }
    return map;
}

Then you can create a map using:

Map<String, String> map = mapOf("One", "1", "Two", "2");

This is not everyone's cup of tea however (because of lack of type-safety) so you could change the implementation to take pairs:

Map<String, String> map = mapOf(pair("One", "1"), pair("Two", "2"));

Where you define pair as a static method that creates a simple object containing two values and then mapOf which converts those pairs into Entries in a Map.

Just saw the Guava ImmutableMap.of() answer (+1). I didn't know that was in API when i wrote the above. — alpian, Apr 26 '11 at 14:57

score 1 · Answer 4 · answered Apr 26 '11 at 14:52

What you have done is normal and common. An alternative is to allow some of the parameters to be null and do some null-checking in the second method. You will have to decide which approach you feel is less ugly.

I have seen APIs where the same method has at least 10 of these parameter-reduced versions for apparently no reason at all. When it balloons up like that, there is something very wrong with your design. Until then, what you have looks fine to me, although I don't really like the Map instantiation much.

score 1 · Answer 5 · answered Apr 26 '11 at 14:53

I'd say that's a pretty common aproach.

The only other (not quite) convenient method I currently could think of would be:

public void redirect(String[]... params)
{
   //build the query string
}

then call

redirect(new String[][]{{"a","1"}, {"b", "2"}} );
redirect();

Note that this uses varargs but it's not quite safe to use (you could pass arrays of length 1 or even 0).

Is there a better way to handle passing in multiple parameters to methods

5 Answers5