How should be my Service method signature?
Asked Answered
P

6

11

I'm using a Service Layer, and until now I used a ServiceObject (which implements ArrayAccess, Iterator, Countable) but I'm wondering if it's a good ideas.

Would you do:

ArticleService::createArticle($articleData, $userId);

or

ArticleService::createArticle(ServiceObject $data);

where $data are:

array(
  'title' => 'Lorem ipsum',
  'body'  => 'Dolor sid amet',
  'userId' => 55,
);

The ServiceObject has the benefit to provide a common Signature for every method, however sometimes it doesn't look efficient, and it is not widely used, it loses its interests.

Any feedback?

Pernell answered 15/5, 2011 at 20:53 Comment(0)
A
5

Change it to:

ArticleService::createArticle($title, $body, $user_id);

This makes it very clear what you need to create an 'Article'.

'option' arrays like your $articleData and $data are impossible to intellisense sensibly and I would advise against it.

Trash your idea for a generic ServiceObject it's actually a very bad idea. Your motives are noble but this is the wrong solution.

If you need more convincing, feel free to poke.

Attainable answered 11/6, 2011 at 23:20 Comment(6)
Thank you for your answer, I understand your arguments, but could you explain a bit why it's so horrible?Pernell
It's 'horrible' because the signature is not really a very useful signature. It just says "this function has input", no shit Sherlock :p You want to know what you should put in there and you'd like to have a really easy way to find out, so why not make it explicit?Attainable
well, article is a simple example, but what if it's with a UserService, a User can have much parameters, I always think that more than 2/3 parameters is a warning of how function is designed. By the way, User is very flexible entity, which may change a lot, so the signature become hard to maintain for backwards compatibility reasons.Pernell
I understand your concern and it's a valid one. Once you start supplying a function with a complex data set (when creating or updating data) you will likely end up with one parameter containing the input data set. What you need additionally is a language to model your input: which properties and what type. Either you take the OO approach and create classes that can hold your data, or you develop a model driven language that can specify what a valid data set should look like.Attainable
In either case your API will likely be very generic (as you suspected). If you're going to do typical CRUD (create, read, update, delete) operations you will likely end up with an API that is very similar to for instance MySQL. What you need to do is leverage your application to a higher level, either using OO modeling or model (data) driven design. In this case my earlier solution is incorrect as my assumption was that you were building a high level API, and not a low-level CRUD API.Attainable
I do not think this is very viable option, if for example, the method is to create a user. I would rather pass as argument an array containing the user data.Estimable
M
2

I think the best approach here would be to have something like

$article = new Article;
$article->title = "Lorem ipsum"
$article->body = "Dolor sit amet"
$article->creator = $userID
ArticleService::createArticle($article)

Though presumably "createArticle" would not be the best name for the function any more, seeing as the Article object was already created. You would probably have something like ArticleService::publishArticle($article) though I don't know what you are making.

What you want is to separate the construction of your data (Article) from the usage of it (ArticleService).

What I'm really trying to say is don't make the argument of createArticle a generic array or "ServiceObject", both of which are useless abstractions of argument lists, make it a class that really represents the relevant entity.

Moralist answered 17/6, 2011 at 18:51 Comment(0)
R
0

I'm vote for the object, as code-completion in complex systems are very good thing and also some auto-validation, as when you know object type you could validate it somehow. But if you don't have complex logic in the createSomething, then array also could fit.

Roughage answered 15/5, 2011 at 22:0 Comment(0)
C
0

The second is way better, you will never have issues with people calling your method the wrong way! It's also much more easier to read and see what is required. For me as a developer it would be clear that I have to pass an object of type ServiceObject and then I start to search for the documentation or an example or interface of this class. But in the first example I have to read the code of createArticle to figure out how both parameters will be handled.

The second approach also will help you to keep your API/signature clean over time. When some additional data is required inside createArticle you simply pass them trough the ServiceObject and you don't need to change the signature!

A little better signature for the first example would be:

ArticleService::createArticle(array $articleData, $userId);

In this case $articleData has to be an array. This prevents errors. In PHP function parameters, only arrays and class names can be used for casting.

Contamination answered 11/6, 2011 at 0:51 Comment(4)
Hi, thank you for your answer. Here is some of my thoughts. I firstly used a ServiceObject because of some optional paramaters, let's say a client doesn't provide a slug (which can be generated automagicly), the idea behind, is I don't want to an isset() on each arguments, so in case it has not been defined, it returns null, entity will handle this correctly, my ServiceObject is nothing more than an implementation of ArrayAccess, Iterator and Count, it is just a plain php object, with a toArray() helper method, but I may be wrong in the way I defined that object.Pernell
Another point about your "learning curve" which is interesting, is if I use a ServiceObject, I'll need to document the option to provided, like 'userId', 'body', etc. while for a classic "signature" I can see method($userId, $body, $title) and it's explicit (but harder in case of an evolution for bc reason). I currently think that I should add PHPDoc explaining which options are available. Also when creating a ServiceObject I do, new ServiceObject(array('userId' => 1, 'body' => 'lorem ipsum'));Pernell
(a lot of comment sorry) another point is when I need only one parameter, like 'getArticleById($id)' it's common to provide an integer here, so is it worth to use a ServiceObject too like getArticleById(new ServiceObject(array('articleId' => 1')); because it respects the consistency but longer to type, and mostly not really usefull, or can I break and use an integer only, or both?Pernell
I'm giving this a -1 because I think it's a bad idea. Please see my suggestion. Feel free to take me up on it.Attainable
I
0

There is no right way, but I would use ArticleService::createArticle($articleData, $userId); in this specific case.

I assume that article have a required non-empty property userId (which you get from the context) and an optional (not very important) content data which may be empty.

This way the connection between the article and the user objects is obvious. The readability is slightly improved. Else you need to be completely familiar with both the data required and the createArticle method.

As proof: instantly when i saw ArticleService::createArticle($articleData, $userId) I understood what the method is doing and what expects for input data, while the second one puzzled me.

Furthermore if userId is missing you will get an error at this point not at the moment you insert it to the database where you probably get an SQL error which is much harder to track.

On the other hand in ZF it is quite common to use arrays as parameters so your code might look in different style.

However it mostly depends of your preference.

Iquitos answered 14/6, 2011 at 16:12 Comment(0)
T
0

I think the better approach is injecting an object. But why is ServiceObject there? You have a method createArticle, then it would be logical to pass an article object to it, no? In that way it is simplier to organize a validation process, you may just mark fields that you want to be validated in annotations.

Also it is a matter of approach you are using. If you use data mapper pattern in your service layers, then you should obviously pass an object to it.

With an object you have transparent and clear interface of communication between your objects. With an array it is not quiet clear what type of data you transfer, what kind of fields and so on.

Tiffanitiffanie answered 17/6, 2011 at 22:44 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.