angularcodereview-com icon indicating copy to clipboard operation
angularcodereview-com copied to clipboard

Change "Is filtering logic handled in controller if needed?" to "Is filtering logic handled in a service if needed?"

Open aj-dev opened this issue 8 years ago • 4 comments

I suggest changing Is filtering logic handled in controller if needed? to "Is filtering logic handled in a service if needed?"

The reason behind this is that controllers should be as lean as possible which makes them easy to test. Services should provide filtered data, ready to be consumed by controllers. It also makes filter functions reusable if they are defined in services.

More info can be found in John Papa's guide Defer Controller Logic to Services and Keep Controllers Focused, Minko Gechev's guide Controllers.

aj-dev avatar Jun 11 '16 08:06 aj-dev

@aj-dev — Thank you, I agree with your reasoning.

The point the performance card is intended to make is:

Do not just blindly perform filtering in the view template using pipes. Instead perform the filtering in the controller so you can better control how many times data needs to be filtered.

You rightfully indicate that it is smart to also delegate filter logic to a service, which wouldn't necessarily help with performance here though.

So I'm wondering if your remark isn't more related to https://github.com/jvandemo/angularcodereview-com/blob/master/src/angularjs/index.jade#L458?

What do you think? Thanks again!

jvandemo avatar Jun 27 '16 12:06 jvandemo

@jvandemo, thanks for you reply.

I partially agree with you.

Controlling how and when data needs to be filtered can be done in a controller but it can also be done and, imho, should be done in a service if we take other best practices into account. Delegating filtering to the service will not degrade performance but will increase testability of the service and keep controller lean.

Proxying controller methods to service might seem like an overhead but if filtering logic is complex (more than 1 line of code), it should probably go into a service.

aj-dev avatar Jul 12 '16 14:07 aj-dev

@aj-dev — I completely agree with your reasoning.

However, the controller will still be needed to access the filter logic in the service, so it's kind of tricky to get the wording right.

How would you phrase it? Thanks for your input!

jvandemo avatar Aug 30 '16 07:08 jvandemo

@jvandemo, it's tricky to phrase it in a concise way, but I think we could simply say "Is filtering logic handled in controller or service if needed?" and add another sentence to the "What?": "Also consider moving complex filtering logic into a service to keep controller lean."

aj-dev avatar Sep 21 '16 11:09 aj-dev