开发者

How do you work with much depending service and dependency injection?

I'm using Dependency Injection with a SOA-like application.

For example, I've a VoteService which is used to cast votes for both Article and Comment.

I've then 4 dependencies, Article, Comment, Database Abstraction Layer, and User which is required to cast votes.

So my constructor has 4 arguments to fill in to get my object.

I always heard that more than 2/3 arguments is a warning of bad code 开发者_StackOverflow中文版design, and I may agree with that.

Maybe my VoteService is then not well designed.

I may have to move the vote casting in both Article & Comment Service?

What do you think?


class ArticleService {
  public function createArticle(); // and other crud methods
}

class VoteService {

  public function __construct($entityManager, $articleService, $commentService, $configurationService);

  // here is the constructor with much arguments

  public function addArticleVote()
  {
     $vote = new ArticleVote();
     $vote->setType(ArticleVote::TYPE_UP)
     $vote->setArticle($article);
     $this->entityManager->persist($vote);
     $this->entityManager->flush();
  }

   // the same method applies for comment
}


I always heard that more than 2/3 arguments is a warning of bad code design

This is not a problem in-and-of itself. It is just an indicator of potential problems.

And this rule is usually applicable to public member functions, not to constructors.

Rules of thumb like this can be dangerous if you apply them blindly. You might have a problem, and you might have no problem at all.

Some examples of problems that might (or might not) exist in your architecture:

  • Your vote service is doing too much, and should be split up
  • Your "vote service" isn't actually its own logical concept, and just an aspect of other concepts in your system
  • You are missing an abstraction of a concept (in this case, a vote, which contains an article, comment, and user)

But I'd really have to see a lot more of your code to tell you if any of them apply. From your short description, it sounds like your service is simple, and that you're over-thinking the problem.

I'm using Dependency Injection with a SOA-like application.

In an SOA application, you will have dependencies that users shouldn't know about. Don't expose them to the users. In this case, users should not know about the DAL.

One way to solve this is to take your application's top layer (the one where you're injecting top-level dependencies), add a shim on top of that, and only expose the shim to the user.

See the Facade Pattern.

Edit:

For example, I've a VoteService which is used to cast votes for both Article and Comment. ... So my constructor has 4 arguments to fill in to get my object.

Dependency injection isn't only constructor injection. You can also inject dependencies through properties (or setter functions), and during each method call. You should use whichever makes sense for each dependency.

Are you always voting on both an article and a comment at the same time? Are you always voting on the same article and/or same comment? My guess is no, so don't pass them in the constructor.

Pass only those dependencies that won't change between uses in the constructor. Pass those dependencies that change on each use in a parameter list in your function. Pass optional dependencies via properties/setters.

In your particular case, you should probably take the $entityManager in the constructor, take the $configurationService either in the constructor or in a setter (depending on if it can live without the dependency), take the $articleService in the addArticleVote method, and take the $commentService in the addCommentVote method.

class VoteService {

  public function __construct($entityManager);

  // Adding this in case you have sensible logic if it isn't present.
  // If it isn't optional (you don't have defaults that make sense to put in this class),
  // then put it back in the ctor
  public function setConfigurationService($configurationService)
  {
    // ...
  }

  public function addArticleVote($articleService) // or $article
  {
    // ...
  }

  public function addCommentVote($commentService) // or $comment
  {
    // ...
  }
}

Maybe my VoteService is then not well designed.

I'd evaluate whether you should even have a VoteService. It might make sense to add vote methods on the ArticleService and CommentService classes, for example, or even on Article or Comment classes that you haven't described.

If you're interested in feedback of this type (seeing how you should refactor your whole set of classes), then you should post another question on http://codereview.stackexchange.com


always heard that more than 2/3 arguments is a warning of bad code design

Completly false. Especially in this case.
You are doing it the right way (DI). No need for changes here imo.

I may have to move the vote casting in both Article & Comment Service?

This could be another solution. But you can freely stay with this imo.
If you want implement this solution you can create an interface IVotable with a method addVote.

At this point you can implements this interface in your Articles/Comments class so you could do:

$a = new Article;
$b = new Comment;
$a->addVote($currentLoggedUser,10); //> Inject only loggedUser (and maybe your DAL)
$b->addVote($currentLoggedUser,10);
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜