In our application we have a number controller actions that have gotten very long and difficult to maintain. Usually #create and #update. A huge reason for this is that the objects handled by the controllers have a bucketload of associations that must be handled according to a complex set of business requirements. In short things have gotten complicated and I''m looking for a way to break up the code in more manageable chunks. I know this is an endeavor as old as programming itself, but I hope people can share some of their experiences. The code i''m dealing with in particular has a lot of error checking. If this & this then fail. If this but not that then fail, and so forth. Of course every conditional has various error messages that must be sent back to client and else clauses and blah blah. It would be interesting to have some kind of callback system, that monitors different stages of the #create workflow for example, and when an error is encountered it bombs out, finds the correct message to display and returns to view. -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/groups/opt_out.
On 18 July 2012 21:00, masta Blasta <lists-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote:> In our application we have a number controller actions that have gotten > very long and difficult to maintain. Usually #create and #update. A huge > reason for this is that the objects handled by the controllers have a > bucketload of associations that must be handled according to a complex > set of business requirements. > > In short things have gotten complicated and I''m looking for a way to > break up the code in more manageable chunks. I know this is an endeavor > as old as programming itself, but I hope people can share some of their > experiences. > > The code i''m dealing with in particular has a lot of error checking. If > this & this then fail. If this but not that then fail, and so forth. Of > course every conditional has various error messages that must be sent > back to client and else clauses and blah blah. > > It would be interesting to have some kind of callback system, that > monitors different stages of the #create workflow for example, and when > an error is encountered it bombs out, finds the correct message to > display and returns to view.The first thing to do is to move most of the logic out into the models. Ideally there should be no business logic in the controllers. Colin -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/groups/opt_out.
Colin Law wrote in post #1069267:> On 18 July 2012 21:00, masta Blasta <lists-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote: >> >> The code i''m dealing with in particular has a lot of error checking. If >> this & this then fail. If this but not that then fail, and so forth. Of >> course every conditional has various error messages that must be sent >> back to client and else clauses and blah blah. >> >> It would be interesting to have some kind of callback system, that >> monitors different stages of the #create workflow for example, and when >> an error is encountered it bombs out, finds the correct message to >> display and returns to view. > > The first thing to do is to move most of the logic out into the > models. Ideally there should be no business logic in the controllers. > > ColinThis is not so much logic as validation. Many, many validations involving multiple models and associations. So if this association parameter is set to this, than this other association param must be set to either this or this otherwise throw error. What you end up with in the end is 100+ lines of if/else statements That''s what makes breaking it up difficult. If I did break things up to other methods or lambdas, i would have these 4-5 line chunks of code that have almost no meaning on their own. Plus since at any point in time I''m juggling 4-5 models, all these would have to be passed around as parameters if some of this logic were to move into ModelClass validations. My one idea so far is to go ahead and break everything into little methods of 4-5 lines and put these in a Module. Then include the module in my controller. then do something like: methodsModule.public_instance_methods.each do |methodName| errors = methodsModule.send methodName, args if errors ...display error ...db:rollback break end end it would not make the code any shorter, but at least somewhat more manageable. -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/groups/opt_out.
On Thursday, July 19, 2012 1:18:54 PM UTC+1, Ruby-Forum.com User wrote:> > Colin Law wrote in post #1069267: > > On 18 July 2012 21:00, masta Blasta <lists-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote: > >> > >> The code i''m dealing with in particular has a lot of error checking. If > >> this & this then fail. If this but not that then fail, and so forth. Of > >> course every conditional has various error messages that must be sent > >> back to client and else clauses and blah blah. > >> > >> It would be interesting to have some kind of callback system, that > >> monitors different stages of the #create workflow for example, and when > >> an error is encountered it bombs out, finds the correct message to > >> display and returns to view. > > > > The first thing to do is to move most of the logic out into the > > models. Ideally there should be no business logic in the controllers. > > > > Colin > > This is not so much logic as validation. Many, many validations > involving multiple models and associations.Isn''t the model the correct place for validations?> So if this association > parameter is set to this, than this other association param must be set > to either this or this otherwise throw error. What you end up with in > the end is 100+ lines of if/else statements > > That''s what makes breaking it up difficult.sniff, sniff, something smells ... maybe it calls for a re-design?> If I did break things up to > other methods or lambdas, i would have these 4-5 line chunks of code > that have almost no meaning on their own.if they have no meaning, then what''s the point of them?> Plus since at any point in > time I''m juggling 4-5 models, all these would have to be passed around > as parameters if some of this logic were to move into ModelClass > validations. >Would they have to be passed around as parameters? The controller receives the params hash that is then used to create or update the model related to the controller, no? If each model validates itself, then when you create the model instance, along with associated models, the validations kick in automatically on save, no? You can then handle any failed validations by returning the error message to the view.> > My one idea so far is to go ahead and break everything into little > methods of 4-5 lines and put these in a Module. Then include the module > in my controller. then do something like: >Would be better if they were included in the Model than the Controller. It can also make testing quicker, by isolating the methods from rails you don''t need to fire up rails to test each method - your are testing right?> > methodsModule.public_instance_methods.each do |methodName| > errors = methodsModule.send methodName, args > if errors > ...display error > ...db:rollback > break > end > end > > it would not make the code any shorter, but at least somewhat more > manageable. > > -- > Posted via http://www.ruby-forum.com/. >(Don''t know much about your whole design, so just made a bunch of assumptions based on the info supplied) BW Paul -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To view this discussion on the web visit https://groups.google.com/d/msg/rubyonrails-talk/-/Q7P8D96v52cJ. For more options, visit https://groups.google.com/groups/opt_out.