6

I've taken over some code at work that has a database repository file that keeps stored procedure names in constants. The file looks like this:

class DatabaseRepository
{
    private const string SP_GET_EMPLOYEES = "GetEmployees";
    private const string SP_GET_DEPARTMENTS = "GetDepartments";
    private const string SP_GET_MANAGERS = "GetManagers";
    // Many more stored procedure constants ...

    public EmployeeList GetEmployees()
    {
        return RunCommand(DatabaseRepository.SP_GET_EMPLOYEES);
    }

    //more methods below that call the stored procedures
}

I find it annoying to deal with these constants when I'm working on the boundary between the code and the stored procedures.

  1. If I want to know what stored procedure a method is using, I have to scroll up or go-to-definition for the constant. I then usually have to scroll back to or search for the method in question because I may be working on that method.

  2. Going the other direction, if I want to know what method calls a given stored procedure (there's usually only one), I have to search the code for the stored procedure name, copy the string constant's name, and then search the code for that string constant.

In both cases #1 and #2, I could prevent searching and scrolling and going-to-definition if the stored procedure names were not constant strings but were actually hard coded as magic strings in the methods themselves.

I like this approach better:

class DatabaseRepository
{
    public EmployeeList GetEmployees()
    {
        return RunCommand("GetEmployees"); //magic string
    }

    //more methods below that call the stored procedures
}

Are there any benefits that I'm not seeing to keeping stored procedure names in constants? I know that it's helpful if more than one method uses that stored procedure. That way, if the stored procedure name changes, I only need to make one change rather than multiple changes. In my experience however, because of their nature, stored procedure names rarely change.

Is it better to have stored procedure names hard coded in magic strings as opposed to kept in constant strings? I know all of the reasons why we should avoid magic strings, but I think this is an exception.

  • In how many places in your code do you call that stored procedure? If you make it difficult to find that information by abstracting it into magic strings, when you make a change to the sproc or want to see how it's data is actually used, you're just creating an additional step for yourself. – TZHX Dec 14 '15 at 16:02
  • @TZHX, almost none of the stored procedures are called from more than one place in the code. In most cases, each stored procedure has just one method that calls it. – user2023861 Dec 14 '15 at 16:10
  • 2
    Then I would put the string with the call. The only reason to have constants is so that your magic strings resolves to a single source, and you already have that. – Robert Harvey Dec 14 '15 at 16:14
  • Don't forget - constants also make your life a lot easier at development-time; you don't get intellisense for hard-coded strings. I'd bet a week's wage at some point you attempt to debug with an error in a hard-coded string somewhere in your solution at some point if this is your approach. – JᴀʏMᴇᴇ Dec 14 '15 at 17:22
2

Short answer: you are fully correct, using constants is not the only possible solution for cases like this, and yours is probably better.

Longer answer: the whole idea of constants fulfills three purposes:

  1. creating a "single source of information" in your program, one and only one place where the names are defined

  2. mapping a string to something more typesafe, which the compiler can check at compile time, and the "intellisense" in your IDE can pick up

  3. making it obvious where to change the code when adding new stored procedures

Given you have exactly one method in the DatabaseRepository class calling the SP, all those methods are listed one after another, and it is the only place where the SPs are called from, then all three purposes are satisfied by your suggestion. Just see this as "a way to define constants for your SP names with different means that the usual const syntax", and not as "magic strings".

Note that the real benefit of using wrapper methods instead of constants comes when your SP have parameters. Then not using constants will keep the full SP signature mapping in one place, namely that function, and not in two places (a constant at the beginning the class, and several hundred lines later a wrapper method).

  • Oh great, an anonymous downvoter, with no explanation what he did not like about my answer. I love those people. – Doc Brown Dec 15 '15 at 10:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.