Question

My web application has a method that parses URL parameters like this.

...
layerName = HtmlPage.Document.QueryString["Layer"] . . . ;
...

One of the departments in our company has a list of URLs with parameters for this application that are difficult to alter for reasons unknown to me. They might use a URL like this. .../default.aspx?Service=Wells&Layer=ActiveWells&Query=XYZ IN ('1234567890...')

Recently, somethings changed similar to the following. The "ActiveWells" layer name changed to "Surface Participation Wells." The "BoreStick" layer name changed to "WellBores." So, the preset URL parameters of that department don't work anymore.

My manager told me to add code that would change any instances of "ActiveWells" to something like "Surface Participation Wells." The manager then said that later when the department with the URL parameters had changed them all to the new names, we could remove that code.

I don't know exactly what "tight coupling" is; but I know it's bad and this sounds like an example of it. It also sounds to me like a bad idea to add code with the intention of keeping it temporarily and removing it later because that code might not ever get removed and become fossilized.

But I followed my orders and I added code like this:

layerName = NameConverter.LayerNameChange(layerName);

There is a switch statement in the static LayerNameChange method.

Months or years from now, whoever is the developer responsible for this app is supposed to know to come in and remove this when that other department has completed altering all there preset URL paremeters.

I suppose another scenerio similar to this would be if a Console or Windows based application had parameters it was expecting for the

Main(string[] args){...}

Is there a better way to be doing this?


Edit:

What if instead of what I stated above, I did something like this pseudocode below.

private void MethodToParseURL_Parameters(Func<string, string> nameReplace)
{
   . . .
   layerName = nameReplace(layerName);
   . . .
}

The calling method would have some sort of,

MethodToParseURL_Parameters(new Func<string, string>(NameConverter.LayerNameChange));

Why should the parsing method need to know about the existence of the NameConverter class?
That's what I asked myself.
After all, that's not part of the responsibility of parsing URL parameters as I see it.

I don't know if I'm overthinking this. I'm new to this level of perception of development. I understand that the question has already been answered, but any further comments on this new idea of mine would be greatly appreciated.

Was it helpful?

Solution

This is not a coupling issue. Passing parameters by name to a function (even a web service) is not particularly a problem.

One problem that has to be dealt with before you go too much further is the SQL Injection security issue that is just ... sitting there. The final parameter in your list is part of a SQL statement, I presume. What happens when someone crafts a fragment of an SQL statement that damages your site? Research "SQL Injection".

You did the right thing, adding the shim NameConverter. It does a translation of the names just the way you are supposed to, and provides a localized, isolated method for remapping names. Suppose another department wants to hold out forever? Your code will stay that way forever. However, I would suggest that you go with a more general map function, using a Map. That way you have cleaner separation between data and control.

As for what your colleague can do in the future? Well, I hope you have a trouble ticketing system, such as Bugzilla or Jira. Just file a ticket for an undefined future release that describes the NameConverter shim and how to change it. With good ticketing discipline, people will be conversant with all outstanding tickets and can recall it when needed.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top