views:

710

answers:

7

I have a very common situation here. And for years I haven't found if what i am doing is RIGHT by industry standards.Consider an application which connects to the database, but where the connection string instead of being stored in some file/setting is being passed as a command line parameter at startup OR the database is browsed to at the time the application starts up.

Well it becomes necessary to save that connection string somewhere within the scope of the app. Most common way I have seen it done is a module or global class with get/set methods to save the connections string. Another way I would do it is using Singleton. Either options my DAL can access the connection string when it needs to through a GetConnectionString method.

Is there a BETTER way of doing this?

Update: i do not have a config file and even if I did I would need the connection string to be read once for the life of the application instance. can you elaborate on the "inject it into any classes" part

+7  A: 

In general global state, be it a global class or a singleton, should be avoided wherever possible.

The ideal solution would be to have your application load up the connection string out of config and inject it into any classes that need it. Depending on the size of your application, an IoC container like Unity or Castle Windsor can help, but certainly isn't a required part of the solution.

If that isn't an option and you're stuck maintaining global state (because of the existing codebase or whatever), I don't know that there's a huge difference between the two approaches you've suggested.

Update: Just to clarify, forget all the stuff about IoC containers for now, "Inject" is just a fancy way of saying "pass in as a parameter" (either to the class's constructor, or via a property, or whatever).

Rather than having a data access class have to ask for the connection string (from some sort of global or a singleton), have it passed in via the constructor or a property.

Update #2: I think there's still some misunderstanding as to what this approach entails.

It basically comes down to whether your data access class looks like:

public class DataAccessClass
{
    public DataAccessClass()
    {
        _connString = SomeStaticThing.GetConnectionString();
    }
}

or

public class DataAccessClass
{
    public DataAccessClass(string connString)
    {
        _connString = connString;
    }
}

These articles (and in fact, many of the articles in that blog) detail a number of reasons why the latter is better than the former (not least because the former is almost impossible to unit test).

Yes, at some place there is going to have to be some static guy responsible for grabbing the connection string in the first place, but the point is that your dependencies on static methods are limited to that one spot (which is likely going to be your Main method in the process of bootstrapping your application), rather than sprinkled throughout your whole codebase.

John Price
-1 This stinks of overdesign.
DannySmurf
No it doesn't. ;)
jalf
@DannySmurf I have to disagree... certainly involving an IoC container on a small project is overkill, but simply refactoring your data access classes to take the connection string as a parameter? How is that overdesigned?
John Price
@[John Price]: this sounds nice in theory, but i don't know of anyone that actually does it this way - do you? if so, how do you do it, esp. since the built-in ADO classes don't directly support it (and some assume config-file connect-strings which makes it even more difficult)
Steven A. Lowe
also, where does the thing that does the 'passing in' get the connect-string? perhaps from a ... static class? total. freakin. overkill.
Steven A. Lowe
@John Price: The IoC stuff is the overdesign, not the config file solution. My problem with this is that even the mention of something that complex in response to a rather simple problem smells like design-pattern-to-oblivion bearing down on the project like a train.
DannySmurf
@DannySmurf I guess I don't see how my second example above (which uses an IoC approach) is any more complex than my first example; it's just a different way of approaching how data gets to where it needs to be and doesn't require any extra code. "IoC" needn't imply "IoC Framework".
John Price
@John Price: I wrote that comment as you were putting your second update in, and I did not see it before I left the page. I agree, there's nothing wrong with that per se.
DannySmurf
this is an excellent solution, if it is possible in the OP's scenario. For example, if one is using out-of-the-box datasets and table adapters, you'd have to go to a lot of trouble to implement this solution - of course, the out-of-the-box DS/TA code assumes the connect-string in the config-file!
Steven A. Lowe
I'm not sure what you mean... even the example code on MSDN for DataSet shows the connection string being passed in from an arbitrary location (in this case, a hard-coded string): http://msdn.microsoft.com/en-us/library/system.data.dataset.aspx
John Price
+3  A: 

If all you're storing is a string (possibly along with a bunch of other global application settings), I would just use a static class with a bunch of properties to hold all that. A singleton is more code & maintenance work, but it's unnecessary work in this case, since your properties class isn't going to DO anything; just hold things.

DannySmurf
I don't think this deserves to be voted down so much. He has a good point, in that plain static data is probably preferable over a singleton. Of course, most of us would go further and remove the global entirely, but as far as the choice between singleton and global goes, I agree with this one.
jalf
You've answered according to the question, however in practice neither solution is the best/better.
Gavin Miller
+1 this answer is correct given the circumstances of the question
Steven A. Lowe
+1  A: 

Sure there is a way. First off, get up to speed on dependency injection and similar techniques, and read about service layer since that's what you need here.

As far as a service layer is concerned, you need some kind of Configuration Service, which will be the module that parts of your application will query for configuration information - connection strings, URIs, etc:

interface IConfigurationService
    string ConnectionString
    bool IntegratedSecurity
    bool EncryptProfiles

It should be pretty straightforward that there is only one instance of IConfigurationService in your system, but this is achieved with a DI container of choice, not with Singleton pattern.

Next, all your DAL services will get a reference to the IConfigurationService:

class FooDal : IFooDal
    IConfigurationService ConfigurationService

so that they can now use IConfigurationService.ConnectionString to get hold of the connection string.

Anton Gogolev
total. freakin. overkill.
Steven A. Lowe
Dude. He asked whether a global or singleton would be better to HOLD A CONNECTION STRING. I can't imagine how adding a "configuration service," along with its own useless interface, for a total of two additional layers of complexity could possibly be the right way to store a string. -1.
DannySmurf
+1  A: 

It depends.

Keep in mind that a singleton:

  • enforces that exactly one instance of the class will exist, and
  • provides global access

A global only provides global access, but makes no guarantees about the number of instantiations.

And the final option, of course, is to use a local variable. That is, pass the config info around as a a parameter to the constructor or wherever it's needed.

Choosing between the first two should be somewhat easy. Is it a disaster if two instances of the class exist? I'd say probably not. I may want to instantiate my own config class to supply separate options to one specific component of the app, or to initialize my unit tests.

There are very few cases where you need to guarantee that exactly one instance will exist. For this reason, a global may be a better choice than a singleton.

The choice between local and global is trickier. Global mutable state is generally best avoided. Immutable state is less of a problem, and won't lead to the same synchronization/concurrency/scalability problems that global mutable state will.

But often, it may be preferable to have it as a local variable you pass around to the components that need it. This can be done manually, simply passing it to the constructor of an object that needs DB access, or it can to some extent be automated with an IoC container.

But in any case, if it is not global, then your code becomes more generic, and more portable. If it has hidden dependencies on classes and global data that must exist for the code to work, then it can't easily be used in other projects, or if you refactor the entire codebase too much.

It also becomes harder to unit test, again because the code won't even compile unless some external data exists that we'd ideally like to leave out of our test.

jalf
A: 

See updated question.

I added some clarification to my answer re: "inject". It doesn't really matter where the connection string comes from (config file or otherwise), just that you pass it into your data access classes, rather than them having to ask some other object for it.
John Price
A: 

as @Anton said, you need to have an interface that exposes something like

interface IConfigurationService
    string ConnectionString

Then whenever one of your class needs your connection string, you provide it with an implementation of IConfigurationService upon construction containing the valid string. You need to find a valid place to create your implementation when you app starts (probably the time where you'll get your database adress), and to pass it along to the class needing it.

Even though it might seems a lot of work compared to singleton or global, this will lower coupling in your code hence improving reusability and make unit testing easier, and is pretty straightforward once you convince yourself that globals are (mostly) evil :)

As mentionned before, there are IoC container that will provide the framework to do that, but it might be overkill in your case, plus its nice to use the pattern for yourself before letting the work to a "magic box"

Axelle Ziegler
+3  A: 

it's nice to see so many creative solutions to such a simple problem ;-)

salient facts, from OP question:

  1. the connect-string is passed on the command-line
  2. many other components may need to use the connect-string

so, there is no way around the use of a static element; whether it's a global variable (hard to do in .NET, really, without an enclosing class), a static class, or a singleton really doesn't matter. The shortest-path solution would be a static class initialized by the Program class that processed the command-line.

Every other solution would still require static access to the passed-in connect-string, though they may hide this behind one or more layers of indirection.

I'm not saying that you don't want to dress up the basic solution with something fancier, but it is not necessary, and it does not eliminate the fundamentally static/global nature of the connect-string as described.

Steven A. Lowe
+1. Not a fan of window dressing, especially on something as simple as app preferences. Adding complexity (and indirection, to hide the fact that one's solution is, in fact, just a global) on top of storage of a string is just silliness.
DannySmurf