search

metridoc-archives / metridoc-grails-core

all grails functionality for metridoc
0 stars 0 forks source link

make InitAuthService do more, change name #23

Closed tbarker9 closed 11 years ago

tbarker9 commented 11 years ago

Controllers that are handling security are getting kind of beefy... maybe we can have a service do more. I think InitAuthService might be a bit too big too. Maybe make a new service called AuthService. Thoughts?

TApicella commented 11 years ago
  1. We already have an AuthService and its pretty chunky. Maybe ControllerAuthService?
  2. The only controller that looks a bit over beefy to me is the AuthController, any others you thinking of?
  3. Everything within InitAuthService seems to logically fit there, except maybe createRole and createRoleName. Even so, I think refactoring InitAuthService would be more confusing than its worth.
tbarker9 commented 11 years ago

AuthService doesn't look that bad to me. I am more ok with big services than big controllers. ControllerAuthService is too confusing to me. Make it sound like a controller instead

tbarker9 commented 11 years ago

Also this is a 0.7 issue, so ignore for now

TApicella commented 11 years ago

Ok. Also, again, if you can give me a list of controllers you think are getting too big that would be great, just so that we are on the same page as far as organizing code goes.

tbarker9 commented 11 years ago

Will do. There are a lot of refactoring targets I would like to target now you are more familiar with grails.

On Fri, Jul 12, 2013 at 1:49 PM, TApicella notifications@github.com wrote:

Ok. Also, again, if you can give me a list of controllers you think are getting too big that would be great, just so that we are on the same page as far as organizing code goes.

— Reply to this email directly or view it on GitHubhttps://github.com/metridoc/metridoc-core/issues/23#issuecomment-20892378 .

TApicella commented 11 years ago

The only thing I'm really seeing here is putting the createRole and createRoleName methods into the role domain object itself, since they really don't belong there.

Also, the Home Service method to create the homepage could be put into InitAuth as well I suppose so that there is only one init service, but that feels unnecessary.

tbarker9 commented 11 years ago

I am just closing this. I don't remember why I opened it in the first place