1
\$\begingroup\$

I have a class with some mandatory properties which I set via constructor, like this (properties omitted for sake of simplicity):

    public SqlDependencyEx(
        string connectionString,
        string databaseName,
        string tableName,
        string schemaName = "dbo",
        NotificationTypes listenerType = NotificationTypes.Insert | NotificationTypes.Update | NotificationTypes.Delete,
        bool receiveDetails = true,
        int identity = 1)
    {
        this.ConnectionString = connectionString;
        this.DatabaseName = databaseName;
        this.TableName = tableName;
        this.SchemaName = schemaName;
        this.NotificaionTypes = listenerType;
        this.DetailsIncluded = receiveDetails;
        this.Identity = identity;
    }

I am trying to use SqlDependencyEx class as a dependency in one of the service like this:

    private readonly IMemoryCache _memoryCache;
    private readonly IAlbumRepo _albumRepo;
    private readonly SqlDependencyEx _listener;
    private readonly string _masterTable = "Armada_Album";
    public AlbumService(IMemoryCache memoryCache, IAlbumRepo albumRepo)
    {
        _memoryCache = memoryCache;
        _albumRepo = albumRepo;
        _listener = new SqlDependencyEx(connectionString: "SERVER=localhost;DATABASE=dbsample;User Id=usr;pwd=cE$6adadZ'[:3;", databaseName: "KEMITEKS", tableName: _masterTable);
    }

The problem is, I don't think it is good to creating a new instance of SqlDependencyEx class in AlbumService class, but at the same time, I want to force consumer of the SqlDependencyEx class to use those properties, since they are all mandatory in order to SqlDependencyEx class to work. So, I don't want to remove those properties and instead of using them in a constructor of SqlDependencyEx class, setting them in separate method etc. I am also using a Ninject as a DI. Is there a better code design to solve this problem, or is it good to leave it as it is?

\$\endgroup\$
2
\$\begingroup\$

If you are using a dependency injection framework then you are not using it correctly. AlbumService's constructor should have in this case another parameter that is the SqlDependencyEx. You should not be creating it by yourself. You are already injecting the other two services. Why not this one too?

And I'm wondering where you have the _masterTable parameter from? I don't see it to be passed to the AlbumService. It has only two arguments.

_listener = new SqlDependencyEx(
    connectionString: "SERVER=localhost;DATABASE=dbsample;User Id=usr;pwd=cE$6adadZ'[:3;", 
    databaseName: "KEMITEKS", 
    tableName: _masterTable);

Another thing is that the name of the SqlDependencyEx does not match the field. Why do you call it a _listener? It's very confusing.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ The reason "Why not this one too" is included as two other services are included, is I will have to resolve those arguments in dependency injection framework. Since, DI framework doesn't support more than one argument, I would like to get rid of them, but still force client to use them. I looked at adapter pattern. It seems it will solve my issue. This code is still in phase of constructing so it is not final. That is why you see _masterTable and bad namings. Some parts are omitted for simplicity. My focus is on removing those arguments while still forcing clients to use them. \$\endgroup\$ – coder Jun 18 '18 at 21:29
  • \$\begingroup\$ @AmelHusic Some parts are omitted for simplicity. please don't simplify your code if you want to receive useful feedback. This always backfires on you and on the answers that aren't as helpful as you had hoped. \$\endgroup\$ – t3chb0t Jun 19 '18 at 8:04
  • \$\begingroup\$ I see your point. Thanks for advice. That is just field like: private readonly string _masterTable = "Armada_Album"; passed to the constructor in SqlDependencyEx class. My whole problem is with SqlDependencyEx class itself and those arguments. \$\endgroup\$ – coder Jun 19 '18 at 8:17
1
\$\begingroup\$

I don't think it is good to creating a new instance of SqlDependencyEx class in AlbumService class

Not necessarily bad and given the hard coded arguments, again, doesn't seem bad.


I want to force consumer of the SqlDependencyEx class to use those properties,

The class should do things, exposing methods; not the properties.


Depending on the client to compose and use these classes and their properties piecemeal indicates missing design elements. I can't say whether a builder or factory pattern or something else is a way to go. However I'm thinking about C# Adapter classes where the various components like Connection can be separately instantiated and composed.

Edit - response to comment

As @t3ch0t says the abbreviated code illustration makes it hard to assess where to go with an answer. So how to abstract away these SqlDependencyEx parameters w/out the design big picture is a guess.

So I guess SqlDependencyEx parameters and then an instantiation w/ hardcoded arguments inside of AlbumService is contradictory. So either AlbumService is subclassed, each with a canned, specific SqlDependencyEx instantiation or we're getting to the idea of the builder pattern. The creation of a particular (top level) object determines what specific participating classes and/or parameters are created; or the instantiations could be thought of as essentially menu choices. I.E. order a certain kind of pizza means certain ingredients, crust types, etc. The builder ensures the appropriate instantiation and composition of these objects.

end Edit

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I probably didn't express myself clearly. By forcing client, I mean using those arguments which are used inside constructor and set them via properties. Sorry about that part. I have separate class which is solely purpose to obtain connectionString, but it doesn't work right now because I am in the process of transforming one of the library from .NET Framework to .NET Core. Will be fix it later. My focus is in possibly better code design by removing those arguments in SqlDependencyEx class. \$\endgroup\$ – coder Jun 18 '18 at 7:38
  • \$\begingroup\$ Before you posted this edit, I added missing field to my original question. So, now when nothing missing, is it possible for one to come up with possible better code design? \$\endgroup\$ – coder Jun 21 '18 at 12:27

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.