search

YouHighFiveMe / youhighfiveme

YouHighFiveMe - a portal to allow people to high five your account
MIT License
2 stars 2 forks source link

Implement a Service Layer #13

Closed artur-gajewski closed 11 years ago

artur-gajewski commented 11 years ago

Instead of putting a lot of code into controllers, use a service layer that contains the business logic. Controllers should delegate the task to a service instead.

janimattiellonen commented 11 years ago

Can we first have some kind of documentation that clearly shows what the service layer and an overall system architect looks like? So that we have a generally agreed system design.

stiggg commented 11 years ago

Services are defined in bundle/Resource/config/services.yml and reside in bundle/Service/ ?

janimattiellonen commented 11 years ago

bundle/Resources/... but yeah.

janimattiellonen commented 11 years ago

Maybe we could use the ACMEDemoBundle to create a proof of concept example on how we could create and utilize the service layer for handling, for example, form requests in a nicely fashion.

I'll grab something to eat and then I could create an example on how I see we could work it out.

artur-gajewski commented 11 years ago

You guys come up with an architecture, I trust you guys as to what the outcome is going to be, so go ahead and add the ACMEDemoBundle which we will use as a guideline.

rdohms commented 11 years ago

I think the use of lambdas makes it very hard to read with little gain.

I have made a suggestion here: http://fixthatcode.com/entry/19/show

stiggg commented 11 years ago

I agree, that lambdas make the logic tedious to follow in code. In principle, trying to keep methods thin and classes do just one thing is a good idea. @rdohms solution looks good to me.

artur-gajewski commented 11 years ago

It seems like rdohms' suggestion is clearer to read and follow, so how about this change in the DemoBundle? https://github.com/YouHighFiveMe/youhighfiveme/blob/mediator/src/Portal/DemoBundle/Controller/DefaultController.php

stiggg commented 11 years ago

Further, I suggest defining mediateForm as abstract, because the precise implementation propably varies between forms. More generally, one service might handle several forms, so even this may not suffice, and we need to define several mediators. This sounds like there's not a simple way to abstract it.

janimattiellonen commented 11 years ago

I say we keep the name processForm() but use rdohms model otherwise. Let's keep it simple and if we later need to change then we do so. We have used processForm() successfully in two projects now so I believe we can start with it.

We don't have to use lambdas and such. We can keep it simple. ;)

The BaseController needs to be moved to another place; either under Portal\MainBundleController or perhaps Portal\Component\Controller namespace.

artur-gajewski commented 11 years ago

EventController now has processForm() function that binds data to the form and then validates it.

Also implemented services for event and highfives.