[OPEN-ILS-DEV] ***SPAM*** osrfAppSessionMakeRequest(): eliminate param_strings?

[Scott McKellar]

In osrf_app_session.c, osrfAppSessionMakeRequest is a thin wrapper for
osrfAppSessionMakeLocaleRequest().  It creates a REQUEST message, sends it
to the specified service, and saves it for future reference so that we
can match it up later with the corresponding response.

There are two ways to specify the parameters for the method being called:

-- You can use the params parameter to pass a pointer to a jsonObject.  In
that case we translate the jsonObject into a JSON string and incorporate
it in the message.  If the jsonObject is a JSON_ARRAY, we pass each element
of it as a separate parameter; otherwise we send the whole thing as a
single parameter.

-- You can use the param_strings parameter to pass an array of pointers,
each one pointing to a pre-formed JSON string.  In that case we pass each
JSON string unchanged as a separate parameter.

----------

I suggest that we eliminate the param_strings parameter altogether,
both here and in the related function osrfAppSessionMakeLocaleRequest().

1. It's redundant.  It doesn't allow you to do anything that you
can't do by other means.

2. Because it's redundant, it's confusing.  In fact natschil was asking
about it yesterday in IRC, which is what triggered this rant.

3. It's never used.  I looked at every call to
osrfAppSessionMakeRequest() in trunk, and every one of them uses params,
not param_strings.

4. It forces us to maintain and document dead code.  In particular,
the function osrf_message_add_param() is never called in any other
context, which means in practice that it is never called at all.

5. As we maintain the code that is never executed, we can't test it
except by building special test drivers.

6. Even if an application has to create the same JSON strings anyway
for other reasons, we don't save anything by trying to reuse them.
In order to add them to the parameter list we translate them into
jsonObjects and then translate those back into JSON.

7. The physical formatting of a message into JSON is a low-level
detail that should be the responsibility of low-level code.  The
upper-level application code should not be concerned with it.

-----------

Migration path:

1. Create a similar function with a different name and no param_strings
parameter.

2. Replace all calls to the old function with calls to the new one.

3. Eliminate the old one.

4. Change the underlying function osrfAppSessionMakeLocaleRequest()
along the same lines.  This will be easy because it's not called from
anywhere else.

5. Eliminate osrf_message_add_param().

All that's different about osrfAppSessionMakeLocaleRequest() is that it
takes an additional parameter for locale, and in practice that parameter
is always NULL.  We could eliminate that parameter as well, but since
it might be useful some day, we might as well keep it around.

------------

Is there any reason to keep the param_strings parameter?

One concern is that somebody (and natschil is the most likely candidate)
might be using param_strings in his or her own code that is not visible
to me in trunk.  If so, this is his or her chance to either prepare or
protest.

Scott McKellar