Hello all, I''m working on a couple of Rails projects at the moment and have noticed something that seems to go against the DRY principal. Why, when scaffolding, are individual CRUD rhtml files created instead of using one as a partial and having the others use that single partial? This especially applies to the list rhtml. I am about to get to the other operations and see how feasible this is, but with the list it seemed to be a very natural fit. This, of course, allows me to style the all list pages in one file. For instance, I wanted to add a row saying "No data found" when there was nothing to list. As I thought about this, I was dreading the upcoming code changes in all the list files, so I created a partial for all the others to utilize. Simple. I''ve heard the arguments (related to other issues) that it''s easy to do, so what''s the big deal? It''s not really a big deal, it just seemed to go against the DRY principle and just wanted to know what the reasoning behind this design was? Have others had the same thoughts? I don''t remember seeing this topic before, so sorry if it has already been introduced. thanks, andy -- Andrew Stone
On 8/18/05, Andrew Stone <stonelists-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> Hello all, I''m working on a couple of Rails projects at the moment and > have noticed something that seems to go against the DRY principal. > Why, when scaffolding, are individual CRUD rhtml files created instead > of using one as a partial and having the others use that single > partial? This especially applies to the list rhtml. I am about to get > to the other operations and see how feasible this is, but with the > list it seemed to be a very natural fit. This, of course, allows me > to style the all list pages in one file. > > For instance, I wanted to add a row saying "No data found" when there > was nothing to list. As I thought about this, I was dreading the > upcoming code changes in all the list files, so I created a partial > for all the others to utilize. Simple. > > I''ve heard the arguments (related to other issues) that it''s easy to > do, so what''s the big deal? It''s not really a big deal, it just > seemed to go against the DRY principle and just wanted to know what > the reasoning behind this design was? > > Have others had the same thoughts? I don''t remember seeing this topic > before, so sorry if it has already been introduced.It doesn''t go against the DRY principle. Scaffolding is code generation and DRY doesn''t apply to generated code. If you want to modify the scaffolding, you could modify the files that generate the scaffolding.
>From the wiki (http://c2.com/cgi/wiki?DontRepeatYourself), the firstline under context is: "Duplication (inadvertent or purposeful duplication) can lead to maintenance nightmares, poor factoring, and logical contradictions." This seems to be a clear case of duplication. So, it would seem that scaffolding does not adhere to the principle and I don''t see why it would not apply to generated code. It would seem logical to me that generated code would not only adhere to the DRY principle, but be an example of the correct way to do so. Maybe I''m just taking this too literally? In any event, code is duplicated and does require me to change multiple files in order to get everything to look/act the same. I don''t want to start a debate such as performance comparisons, seen enough of that here :) I just would like to know if there was a specific reason for this duplication. thanks, andy -- Andrew Stone
On 8/18/05, Andrew Stone <stonelists-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> >From the wiki (http://c2.com/cgi/wiki?DontRepeatYourself), the first > line under context is: > > "Duplication (inadvertent or purposeful duplication) can lead to > maintenance nightmares, poor factoring, and logical contradictions." > > This seems to be a clear case of duplication. So, it would seem that > scaffolding does not adhere to the principle and I don''t see why it > would not apply to generated code. It would seem logical to me that > generated code would not only adhere to the DRY principle, but be an > example of the correct way to do so. > > Maybe I''m just taking this too literally? In any event, code is > duplicated and does require me to change multiple files in order to > get everything to look/act the same. > > I don''t want to start a debate such as performance comparisons, seen > enough of that here :) I just would like to know if there was a > specific reason for this duplication.Dunno about the performance stuff, but generated code is generally not considered a violation of DRY. Essentially, DRY states that there should be one authorative place for all knowledge. In the case of scaffolding, that one authorative places is the code that generates the scaffolding. If you update that code, then you can regenerate the scaffolding and everything is updated.
Joe Van Dyk wrote:> Dunno about the performance stuff, but generated code is generally not > considered a violation of DRY. Essentially, DRY states that there > should be one authorative place for all knowledge. In the case of > scaffolding, that one authorative places is the code that generates > the scaffolding.It''s a good idea to minimize duplication everywhere. > If you update that code, then you can regenerate the > scaffolding and everything is updated. The point of scaffolding is to be customized though. Regenerating scaffolding would remove all your intentional changes. Until scaffolding is able to diff the generated and existing code, and allow the user to merge it, we''d best not rely on updated generators a means to update your code.
Joe Van Dyk wrote:>Dunno about the performance stuff, but generated code is generally not >considered a violation of DRY. Essentially, DRY states that there >should be one authorative place for all knowledge. In the case of >scaffolding, that one authorative places is the code that generates >the scaffolding. If you update that code, then you can regenerate the >scaffolding and everything is updated. >I''d have to say that I definately disagree. Generated code is very different from using a third party library. It is code which is actually in your project and is designed to be understood and potentially modified by you. If it is modified, then by anyone''s standards it quickly becomes no different to any other code in your application and duplication should be factored out where possible. If it is not modified, then you need to ask yourself, why is it generated (rather than being an external library)? I think it''s great that rails has two ways of scaffolding. One, by using the mixins, or two by running the generators. Personally, I use the generators, because I know that although the scaffolding is handy to get me started, I wont want to keep it the same forever. So, once the generator has been run, I treat the code it creates as part of my project, and factor out any duplication as it occurs. I did much the same thing in with the login generator. However, I am a little worried that perhaps we rails folk are using generators a bit too often, in situations where we don''t want people to modify the code on every site they use, but instead want to be able to quickly upgrade as newer versions of this ''feature'' are released. This is what libraries are for, and we shouldn''t forget that. Craig
On Thursday 18 August 2005 08:15 pm, Joe Van Dyk wrote:> [...] Essentially, DRY states that there > should be one authorative place for all knowledge. In the case of > scaffolding, that one authorative places is the code that generates > the scaffolding. If you update that code, then you can regenerate the > scaffolding and everything is updated.There are two types of generated code. Code that is always generated on each build and the generated code is treated as a transient product (i.e. not stored in source control). I''m not particularly worried about DRY in that kind of generated code. However, the scaffolding seems to be the other kind where it is generated once and then maintained manually. Here, DRY is just as important in the generated code as in regular manual code. There is a third kind of generated code that establishes guards (generally marked with special comments) around the areas of customization. Regenerating the code replaces the generated stuff of keeps the customizations. I personally consider this third kind of generated code to be the spawn of the devil. -- -- Jim Weirich jim-Fxty1mrVU9GlFc2d6oM/ew@public.gmane.org http://onestepback.org ----------------------------------------------------------------- "Beware of bugs in the above code; I have only proved it correct, not tried it." -- Donald Knuth (in a memo to Peter van Emde Boas)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 I guess this link is what you are looking for. Although, it does not patch any of the code generation though... http://dereksivers.com/rails-shared-controller.html Regards, - ------------------------------------------------------------------------ /*Ronny Hanssen*/ Andrew Stone wrote:> Hello all, I''m working on a couple of Rails projects at the moment and > have noticed something that seems to go against the DRY principal. > Why, when scaffolding, are individual CRUD rhtml files created instead > of using one as a partial and having the others use that single > partial? This especially applies to the list rhtml. I am about to get > to the other operations and see how feasible this is, but with the > list it seemed to be a very natural fit. This, of course, allows me > to style the all list pages in one file. > > For instance, I wanted to add a row saying "No data found" when there > was nothing to list. As I thought about this, I was dreading the > upcoming code changes in all the list files, so I created a partial > for all the others to utilize. Simple. > > I''ve heard the arguments (related to other issues) that it''s easy to > do, so what''s the big deal? It''s not really a big deal, it just > seemed to go against the DRY principle and just wanted to know what > the reasoning behind this design was? > > Have others had the same thoughts? I don''t remember seeing this topic > before, so sorry if it has already been introduced. > > thanks, > andy-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.0 (MingW32) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDBV5LMRRzQX3ma+kRAl+bAJwN7Fyw/c0w7GyTfNBwjvHpLdAxTACfdH9l KCli4vep8NqFTvnWax5mkbE=smn0 -----END PGP SIGNATURE-----
I thought I would just demonstrate what I was doing for those interested in using the same approach for the list pages. It''s really easy, but may save some time for some people by not having to figure out anything... This is for a ticket tracking system I''m writing that will be opensourced. I brought this up a couple of months ago on this list. This is the beginning of a system that will include an inventory component (among other things) that we will implementing on our intranet. Note: I don''t use the pluralization design for my tables. Setup (using a project table for example): Table Definition (postgres): #begin -- Project definition create table project( id serial primary key, name varchar(40) not null, updated_on timestamp default current_timestamp, updated_by integer references updater(id) )without oids; #end Note: the updater table is defined as: -- This table is used as the foreign key for all history,note,etc.. -- This enables members (users) to be added/removed without breaking the -- foreign key relationships. This is particulary useful when -- importing data from another system and wanting to purge invalid -- members (users). create table updater( id serial primary key, name varchar(80) not null )without oids; ./script/generate/scaffold project create: app/views/shared/_list.rhtml (attached) modify: app/views/projects/list.rhtml #begin <% Cls = Project @l_items = @projects @l_pages = @project_pages %> <%=render(:partial => ''/shared/list'')%> #end Note: remove all other code from the list.rhtml add to app/models/project.rb: #begin def self.list_columns columns.reject {|c| case when c.name == "name" : false else true end } end #end I created the list_columns to restrict what I wanted to display on the list page as the content_columns displayed too much information. Sorry for the lengthy email, just thought others may want to see this. -andy -- Andrew Stone _______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
Thanks for sharing. I like your solution better than the one I wrote. I will use yours from now on. I did make a small change to the self.list_columnsdefinition. To me, this is better semanticly: def self.list_columns columns.reject {|c| unless %w(first_name last_name email).include? c.name <http://c.name> true end } end BTW. how do you turn off pluralization? It''s becoming difficult to remember when to use singular and when to use plural references. Cheers. On 8/19/05, Andrew Stone <stonelists-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> > I thought I would just demonstrate what I was doing for those > interested in using the same approach for the list pages. It''s really > easy, but may save some time for some people by not having to figure > out anything... > > This is for a ticket tracking system I''m writing that will be > opensourced. I brought this up a couple of months ago on this list. > This is the beginning of a system that will include an inventory > component (among other things) that we will implementing on our > intranet. > > Note: I don''t use the pluralization design for my tables. > > Setup (using a project table for example): > > Table Definition (postgres): > > #begin > > -- Project definition > create table project( > id serial primary key, > name varchar(40) not null, > updated_on timestamp default current_timestamp, > updated_by integer references updater(id) > )without oids; > > #end > Note: the updater table is defined as: > -- This table is used as the foreign key for all history,note,etc.. > -- This enables members (users) to be added/removed without breaking the > -- foreign key relationships. This is particulary useful when > -- importing data from another system and wanting to purge invalid > -- members (users). > create table updater( > id serial primary key, > name varchar(80) not null > )without oids; > > ./script/generate/scaffold project > > create: > app/views/shared/_list.rhtml (attached) > > modify: > app/views/projects/list.rhtml > > #begin > > <% > Cls = Project > @l_items = @projects > @l_pages = @project_pages > %> > <%=render(:partial => ''/shared/list'')%> > > #end > Note: remove all other code from the list.rhtml > > add to app/models/project.rb: > > #begin > > def self.list_columns > columns.reject {|c| > case > when c.name <http://c.name> == "name" : false > else true > end > } > end > > #end > > I created the list_columns to restrict what I wanted to display on the > list page as the content_columns displayed too much information. > > Sorry for the lengthy email, just thought others may want to see this. > > -andy > > -- > Andrew Stone > > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails > > > >-- Best Regards, -Larry "Work, work, work...there is no satisfactory alternative." --- E.Taft Benson _______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
On 8/19/05, Larry Kelly <ldk2005-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> Thanks for sharing. I like your solution better than the one I wrote. I > will use yours from now on. I did make a small change to the > self.list_columns definition. To me, this is better semanticly: > > def self.list_columns > columns.reject {|c| > unless %w(first_name last_name email).include? c.name > true > end > } > end > > BTW. how do you turn off pluralization? It''s becoming difficult to > remember when to use singular and when to use plural references. > > Cheers.You''re most welcome and thanks for change recommendation. I like your way much better. You can turn off pluralization in config/environment.rb by adding: ActiveRecord::Base.pluralize_table_names = false This doesn''t change the pluralization of the controllers (there is a thread related to this topic already). It does allow the models to work with singular table names. Thanks again for the feedback. -andy -- Andrew Stone