Best Practices on Code Duplication c#

Question

I am trying to structure my code in such a way to reduce/avoid code duplication and I have encountered an interesting problem. Every time my code invokes a stored proc, I need to pass few variables that are common to the stored proc: such as username, domain, server_ip and client_ip. These all come from either HttpRequest object or a system.environment object.

Since these are passed to every stored proc, my initial thought was to create a utility class that is a database wrapper and will initialize and pass these every time, so I don't have to do it in my code. The problem is though that c# class (inside App_Code folder) doesn't see Httprequest object. Of course, I could pass this as an argument to the wrapper, but that would defeat the whole purpose of creating the wrapper. Am I missing something here?

I realize it's not such a huge deal to repeat 4 lines of code each time I call a stored proc, but I would rather eliminate the code duplication at the very early stages.

Answer 1

HttpContext.Current has similar information to what you find in HttpRequest and more importantly is available inside App_Code.

Answer 2

I would keep it the way you have it now. It's cleaner, easier to extend/modify, and easier to unit test.

As for using HttpContext instead as some others have suggested, I would say that it is a bad idea. Once you start introduce dependencies in your domain on HttpContext, it's very difficult to take it out. What if later on you wanted to use your module without an HttpContext? What about unit testing it?

Answer 3

Try System.Web.HttpContext.Current.Request to get the current request.

Answer 4

Here's a weird idea you may or may not like: define a 'profile' class and a function that expands the profile into the arguments of functions taking the common arguments.

class P {
    readonly string name;
    readonly string domain;
    public P(string name, string domain) {
        this.name = name; this.domain = domain;
    }
    public void inject(Action<string, string> f) {
        f(p.arg1, p.arg2);
    }
    public T inject<T>(Func<string, string, T> f) {
        return f(p.arg1, p.arg2);
    }
}

It might work better in VB.net where you have the AddressOf operator. I would be really cautious using this type of thing, because you could easily damage readability and encapsulation.

Answer 5

As a general rule regardless of programming language, if you can squint your eyes and the code looks the same you should make a function/method/message out of it and pass the parameters.

Another thing to look at once you have methods that take a large number of parameters (4 is a good rule of thumb, but it is definatly a case-by-case basis) it is time to make that method take an object as a parameter instead of individual parameters. 99.99999999999999999999% of the time such an object should be immutable (no writeable instance variables).

Answer 6

You are possibly headed down a slippery slope. The point to DRY is to not repeat business logic in multiple places where a change in requirement creates the need to change code in multiple similar places. You don't necessarily refactor just because 4 lines are the same if those 4 lines are context dependent. You have also broken encapsulation by referencing the httprequest in that you are using a global variable. As a consumer of you class I would have to know the implementation detail that I could only call you from a web application.

That being said, if you take that into account and still want to proceed, here is another option for information like this. Create a custom SecurityPrincipal (Implement IPrincipal) that contains the properties you need and attach it to the thread. Fill them when the user logs in and then you can access it anywhere during the request. Your caller would still need to make sure this was done but at least it isn't platform specific.

Otherwise for the best encapsulation, pass in a class with the properties you need into the constructor for each object that needs to consume those properties.

Answer 7

Set up your data layer to inherit from a base class which contains 4 properties for those values. Make the public constructor require those 4 properties.

Then do something similar in the business layer - base class with those 4 properties in the constructor.

Then the UI does new BusObj( Request["username"], ... ).method()

Within the data layer you can have a method that builds a SQLParameter array with those 4 properties, then each method can add additional parameters to the array.