tag:blogger.com,1999:blog-1042801964488488185.post8234329961527935333..comments2023-05-23T06:48:28.455-05:00Comments on Jake Scruggs: I'm Thinking of Putting View Logic into a ModelJake Scruggshttp://www.blogger.com/profile/16274380203959781950noreply@blogger.comBlogger21125tag:blogger.com,1999:blog-1042801964488488185.post-86886941052979629532008-08-31T13:12:00.000-05:002008-08-31T13:12:00.000-05:00What if your models get used in multiple applicati...What if your models get used in multiple applications - which happens to us all the time? <BR/><BR/>I've thought of doing this, and have run into the problem before. You end up with methods that only apply to one rails app. Ick. <BR/><BR/>What about a decorator for that object? You isolate the view logic into its own class, and remove the switch. I don't have the sample code - but you can probably picture a CarViews object (or something with a better name) that uses method missing to delegate to the real car when it needs it.paytonruleshttps://www.blogger.com/profile/15293750156104975564noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-14171088591130486412008-08-26T22:23:00.000-05:002008-08-26T22:23:00.000-05:00My response:http://rhnh.net/2008/08/27/no-view-log...My response:<BR/><A HREF="http://rhnh.net/2008/08/27/no-view-logic-in-models" REL="nofollow">http://rhnh.net/2008/08/27/no-view-logic-in-models</A><BR/><BR/>summary: you can use a better data structure to solve your problemAnonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-80972255795786882452008-08-26T22:21:00.000-05:002008-08-26T22:21:00.000-05:00I get scared anytime I see a million elsif stateme...I get scared anytime I see a million elsif statements. Moving it to a helper or a view doesn't make it go away. I don't think its that big of a deal to stick this kind of info in the model. You should never subscribe to rules so hard that it makes you write nasty code.Unknownhttps://www.blogger.com/profile/05935783323884046530noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-44957419294862232462008-08-26T19:05:00.000-05:002008-08-26T19:05:00.000-05:00Thing is, there's a difference between "class whic...Thing is, there's a difference between "class which persists its data in a database" and "class which forms part of the model layer of an application".<BR/><BR/>The use of ActiveRecord tends to make you think of these two as one and the same, but they're not. Sometimes the UI layer needs to persist UI settings in a database. Shock f---ing horror. MVC cries in pain.<BR/><BR/>If there's a genuine 'model level' concept which you model and persist and use to infer the relevant UI state, then great.<BR/><BR/>But sometimes, you'll just need to persist something blatantly UI-level, like 'x and y coordinates of UI widget foo' in the database. And that's fine. It would be nicer if you were using a component-based web framework though, which doesn't force you to put anything database-persisted in a 'model' layer.Matthhttps://www.blogger.com/profile/07759263441119052158noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-73952333842232003352008-08-26T11:35:00.000-05:002008-08-26T11:35:00.000-05:00Which is more Terrible, the violation of MVC or th...Which is more Terrible, the violation of MVC or the Switch on Object, ActsAsFlinn?Jake Scruggshttps://www.blogger.com/profile/16274380203959781950noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-60032261309169508802008-08-26T10:17:00.000-05:002008-08-26T10:17:00.000-05:00terrible.terrible.Anonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-50982000755109365142008-08-25T23:52:00.000-05:002008-08-25T23:52:00.000-05:00Hi Jake, What you are saying strikes a chord. If w...Hi Jake, <BR/>What you are saying strikes a chord. If we look at your code it looks like the presentation very much depends on the model or the type of the model. So it totally makes sense to use polymorphism to figure this out and replace those ugly if/else/switch statements. Once we have crossed that hurdle it looks like a view or a presenter class becomes your first refactoring as you and some commenters suggest. I have a weird feeling about putting all this in helpers, since helper itself does not sound very OO. <BR/>Once we have this first refactoring in place we have done some damage control. But what you are referring to(as pushing under the rug) needs a deeper thought and look at the code. At this point we should really start asking the question - how much is the view dependent on the type of the model? Well if we take a closer look - we find that the model defines the way it is going to look. So the behavior certainly belongs into the model.<BR/>But does this relationship really warrant extending the ActiveRecord class? and does it justify putting this code into the model(Statically)? Maybe not. What could make it better is a mixin? Based on the model type we should include the mixin for the appropriate layout like so<BR/>---- Code begins ----<BR/><BR/>module Ford<BR/> def layout<BR/> "ford"<BR/> end<BR/> <BR/> def stylesheets<BR/> ['ford_layout.css', 'ford_theme.css']<BR/> end<BR/> <BR/> def scripts<BR/> "discount.js"<BR/> end<BR/>end<BR/><BR/>class Car < ActiveRecord::Base<BR/> .<BR/> .<BR/> .<BR/> <BR/> def present<BR/> extend self.type.to_s.Constantize<BR/> end<BR/> .<BR/> .<BR/> .<BR/>end<BR/><BR/>class CarController < ApplicationController<BR/> @car = Car.find(params[:id])<BR/> @car.present<BR/> render :action => find_view, :layout => @car.layout<BR/>end<BR/>---- Code ends ------<BR/>Using mixins really makes this code look like its meant to be in ruby.<BR/><BR/>-SudhindraSudhindra Raohttps://www.blogger.com/profile/07437146809844632919noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-51464685182572757472008-08-25T19:59:00.000-05:002008-08-25T19:59:00.000-05:00It's interesting to me that many commenter suggest...It's interesting to me that many commenter suggest moving the big switch/if/else into a helper or presenter. Which seems to me a bit like sweeping the dirt under the rug.<BR/><BR/>While moving from switch to OO is such a classic bit of Refactoring it's hard to dismiss. <BR/><BR/>It is a violation of the MVC layers, but what bad thing happens if I put stylesheet, layout, and javascript file information in the model? Well if the name of any of those files changes, I guess I'll have to change the model. Which doesn't seem like it will happen very often. <BR/><BR/>However, it does set a bad precedent -- future coders may start jamming more and more view stuff into the model and that would be bad.<BR/><BR/>When is it OK to break the rules? Or, to put it another way, if I have to choose between switching on class or breaking MVC which is more egregious? I guess it depends on the team who will be maintaining the code.Jake Scruggshttps://www.blogger.com/profile/16274380203959781950noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-59838973589059282472008-08-25T06:53:00.000-05:002008-08-25T06:53:00.000-05:00Seems like you could just use a big data structure...Seems like you could just use a big data structure and extract it to a different class. <A HREF="https://gist.github.com/7062/71ec7c0b47354cc3a06b39974f44c31ae9726a19" REL="nofollow">Here is a try on gist</A>.Dave Hooverhttps://www.blogger.com/profile/11416140599591449857noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-57813534437585964812008-08-25T04:21:00.000-05:002008-08-25T04:21:00.000-05:00Why not remove it from your controller, and put it...Why not remove it from your controller, and put it in the view where it belongs using a helper which is perfect for this situation.<BR/><BR/># some_helper.rb<BR/>def place_stylesheets(car)<BR/> if car.is_a?(Toyota)<BR/> ...<BR/> elsif car.is_a?(Ford)<BR/> ...<BR/> end<BR/><BR/>end<BR/><BR/>That can concatenate the scripts and stylesheets properly right in the view where they belong. Now for the layout. That is a bit trickier. Myself, I would perhaps move that to a simple method in a helper or as a private method in the controller.nesquenahttps://www.blogger.com/profile/16240897807621094760noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-31006642765279382022008-08-25T03:35:00.000-05:002008-08-25T03:35:00.000-05:00how about a helper that keeps a hash of model_clas...how about a helper that keeps a hash of model_class => class_specific_configuration - the class_specific_configuration could either be a hash like {:stylesheets => 'toyota.css' ....}, an openstruct or maybe a small CarViewConfig class. you could even create a hierarchy of CarViewConfig classes if you want - without putting any view logic into your actual models.Anonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-38622008831556591132008-08-25T03:29:00.000-05:002008-08-25T03:29:00.000-05:00Or you could use YAML files to hold the various co...Or you could use YAML files to hold the various configurations, something like this...<BR/><BR/>config/cars/toyota.yml<BR/>config/cars/ford.yml<BR/>etc.<BR/><BR/>@car_config = YAML.load(File.read(Rails.root + "/config/cars/" + @car.class.name.underscore + ".yml"))stefanohttps://www.blogger.com/profile/08821085074057948313noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-23602098314751496192008-08-25T03:25:00.000-05:002008-08-25T03:25:00.000-05:00That looks digusting!I would just create a partial...That looks digusting!<BR/><BR/>I would just create a partial (or view) for each car subclass with fallback to some default (e.g. two_columns). In controller you just check if custom view exists or fallback to default one.Anonymoushttps://www.blogger.com/profile/00272049513122973998noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-41616298062404091872008-08-25T01:46:00.000-05:002008-08-25T01:46:00.000-05:00I think your original is much better than putting ...I think your original is much better than putting view code in the Model, which is lazy. <BR/><BR/>There are many better ways of doing it, use your imagination, for example you could create a hash with the required objects for each model.<BR/><BR/>foo[model.class][:javascript]<BR/><BR/>I don't mean to sound harsh, but if you really do need to put view code in your model it means your design is broken.Anonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-40851689636133694742008-08-25T01:33:00.000-05:002008-08-25T01:33:00.000-05:00id use a helper method which reads data form layou...id use a helper method which reads data form layouts.ymlAnonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-15927433583342648742008-08-25T01:28:00.000-05:002008-08-25T01:28:00.000-05:00What about using the class name to determine the s...What about using the class name to determine the stylesheet to load? Rather than specify the stylesheets, how about creating a:<BR/><BR/>car.js<BR/>ford.js<BR/><BR/>car_layout.css<BR/>ford_layout.css<BR/><BR/>In your view you could use:<BR/><BR/>stylesheet_link_tag @car.class.to_s<BR/><BR/>If this is not possible or would create too much complication, it still doesn't make sense to define the stylesheets and JavaScrips as instance methods. I would probably opt for a module that lets you define class-level attributes. Each car instance does not need its own stylesheet, but I would assume that each type of car (car class) has its own set of view components. If I had to do this, I would opt for something like the following:<BR/><BR/>class Ford<BR/> stylesheet 'ford'<BR/> layout 'ford_two_column'<BR/>endAnonymoushttps://www.blogger.com/profile/10396904220533470356noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-52642422039858016532008-08-24T23:55:00.000-05:002008-08-24T23:55:00.000-05:00I most of this belongs in a helper actually. Ex: i...I most of this belongs in a helper actually. <BR/>Ex: in yer view<BR/><BR/>= stylesheets_for @car<BR/>= scripts_for @car<BR/><BR/>And your code can be shortened by having an conditional for every scenario, rather than every type.<BR/><BR/>ex:<BR/><BR/>case @car<BR/>when Ford<BR/> return ''<BR/>when Toyota, Saturn<BR/> return 'two_column'<BR/>when Hyundai<BR/> return 'three_column'<BR/>else<BR/> raise "#{car.class} doesn't have a layout specified"<BR/>endWesley Moxamhttps://www.blogger.com/profile/02295024765892049282noreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-7897303448841030542008-08-24T23:25:00.000-05:002008-08-24T23:25:00.000-05:00This could easily be refactored to fit into the MV...This could easily be refactored to fit into the MVC pattern better.<BR/><BR/>I tried to type it here but couldn't figure out how to get Blogger to accept erb tags.<BR/><BR/><A HREF="http://pastie.org/259253" REL="nofollow">Here</A> is the sample code.Anonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-9643151978994296782008-08-24T22:56:00.000-05:002008-08-24T22:56:00.000-05:00Not sure of the best way to do this w/ javascript,...Not sure of the best way to do this w/ javascript, but for stylesheets, why not just have stylesheets that use import statements. So you could have toyota.css which imports two_column_layout.css and two_column_theme.css.<BR/><BR/>Layouts could be named for each make as well, each including the appropriate partials.<BR/><BR/>Now the view logic is not only not in the model, it's not even in the controller :)Anonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-44415447289986614182008-08-24T21:49:00.000-05:002008-08-24T21:49:00.000-05:00What about using the Presenter Pattern?http://blog...What about using the Presenter Pattern?<BR/><BR/>http://blog.jayfields.com/2007/03/rails-presenter-pattern.html<BR/><BR/>JustinAnonymousnoreply@blogger.comtag:blogger.com,1999:blog-1042801964488488185.post-91687671815003352482008-08-24T21:07:00.000-05:002008-08-24T21:07:00.000-05:00If you have classes for each car type like that, t...If you have classes for each car type like that, then absolutely.Mr. Neighborlyhttps://www.blogger.com/profile/02063939073935274480noreply@blogger.com