Hey, I have a system that I am trying to design, it contains news articles, events, etc... There is model representing articles, one representing events, etc... Articles or events can be active or inactive, so I can do a query to return all active articles, all articles or all inactive articles. The question that I have is what would be the best way to design this. Right now I have each method (such as recent articles, featured articles, etc...) taking an ''active'' parameter. This requires littering the code with method calls taking this parameter. I have also thought about setting a class variable on the models to set whether active/inactive/all articles are returned, so that anytime you want to set the type returned you set this class variable and then call your functions (recent_articles, etc...). What I would like to know is if there are any suggestions as to a good practice to follow in how to implement this functionality, one of the ways I have stated or some other way that I haven''t considered. I hope I have been clear. Thanks for the help in advance. -- Posted via http://www.ruby-forum.com/.
Ditto, same question. Seems like there are 3 pretty obvious ways to do it... But... Is there a Ruby (e.g., elegant) Way to do this? -- Posted via http://www.ruby-forum.com/.
On 6/16/06, Laz <stroke_it@hotmail.com> wrote:> > Hey, > > I have a system that I am trying to design, it contains news articles, > events, etc... There is model representing articles, one representing > events, etc... > > Articles or events can be active or inactive, so I can do a query to > return all active articles, all articles or all inactive articles. > > The question that I have is what would be the best way to design this. > Right now I have each method (such as recent articles, featured > articles, etc...) taking an ''active'' parameter. This requires littering > the code with method calls taking this parameter.Use a method call with the ''active'' parameter set to default def recent_articles( since = 5.days.ago, is_active = true ) that way when you call recent_articles if you don''t specify a parameter it will use true and 5 days ago I have also thought about setting a class variable on the models to set> whether active/inactive/all articles are returned, so that anytime you > want to set the type returned you set this class variable and then call > your functions (recent_articles, etc...).This would be very bad when multiple requests are being served I think. Also you would still have to set the condition in each call to a method since you won''t know what state the class variable will be in. I would stay away from this one. What I would like to know is if there are any suggestions as to a good> practice to follow in how to implement this functionality, one of the > ways I have stated or some other way that I haven''t considered. > > I hope I have been clear. Thanks for the help in advance. >--> Posted via http://www.ruby-forum.com/. > _______________________________________________ > Rails mailing list > Rails@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/railsI''m not 100% sure that I understand your problem but..... I''m assuming that something has recent articles, and active articles etc. I''ll assume it''s news ie news has_many articles So I think if you wanted to call news.articles and only return open ones To do this, in the News class has_many :articles, :conditions => { :active => true } # get only active articles with news.articles has_many :all_articles, :class_name => "Article" # get all articles associated with news with news.all_articles has_many :inactive_articles, :class_name => "Article", :conditions => {:active => false } # has_many :recent_articles, :class_name => "Article", :conditions => "active = true AND published_at > #{5.days.ago}" # news.recent_articles I haven''t tested this so I''m not sure that it will do what you want but I think it should. Alternatively you could make some class methods on your articles or instance methods on your News class etc class Article < ActiveRecord::Base class << self def recent_articles find( :all, :conditions => "published_at > #{5.days.ago}" ) end end ... end Call this via Article.recent_articles -------------- next part -------------- An HTML attachment was scrubbed... URL: http://wrath.rubyonrails.org/pipermail/rails/attachments/20060617/0e31df21/attachment.html
Is there a newsgroup more suited to this question that I should have posted it or is this one ok? -- Posted via http://www.ruby-forum.com/.
Thanks Daniel, Yea, the first examples you showed is the way I''m basically doing it now, except not using true or false, because I need 3 options (active, inactive, all). Your second comment about having to keep track of the state of the object (i.e. whether it is in active/inactive/all state) was a point bothering me in that implementation, but I was wondering if the state monitoring would be offset by the fact that most of the time when these methods are called, it is within a small method itself, therefore just setting the state at the beginning of that method may not have been hard to keep track of. Any thoughts on this? Your third suggestion looks interesting, I hadn''t thought of that, have to give that one some thought now. Thanks for the reply. If you have any more thoughts on what I said or any others, don''t hesitate. I''m trying to broaden my programming skills by learning from best practices and getting good feedback. Thanks again Daniel. -- Posted via http://www.ruby-forum.com/.
On 6/17/06, Laz <stroke_it@hotmail.com> wrote:> > Thanks Daniel, > > Yea, the first examples you showed is the way I''m basically doing it > now, except not using true or false, because I need 3 options (active, > inactive, all). > > Your second comment about having to keep track of the state of the > object (i.e. whether it is in active/inactive/all state) was a point > bothering me in that implementation, but I was wondering if the state > monitoring would be offset by the fact that most of the time when these > methods are called, it is within a small method itself, therefore just > setting the state at the beginning of that method may not have been hard > to keep track of. Any thoughts on this?Setting the state on an instanciated object is no problem. That status effects only that object. You could then put an instance method on it to query its state. But putting the state as a class variable will mean that if every instance of that object sets the class variable and you read a couple of hundred instances of article from your db you could be in all sorts of strife... especially if there is someone else reading those objects and changing their state at the same time. If you check the state in the method, then go to use it, it may have changed you just don''t know. Class variables are good for things like allowable states. @@states = ["active","inactive", "something else"] in you db table (and with rails therefore an instanciated object) for Articles you presumably have a column called status. You can determine what that status is set to by using @my_article.status This will return the status of the article. You could also define an instance method that looks more like a question def is_active? self.status == "active" end This will return true if the status of the article is set to "active" by a call to @my_article.is_active? The things that you can do with this are very flexible. Say you set up the is_active? method above and you have a collection of Articles in a variable. say you used articles = Articles.find(:all ) You can split these into active and inactive by using active_articles, inactive_articles = articles.partition{ |a| a.is_active? } Partition is a method of the Enumerable module. Then you could select recent articles by recent = active_articles.select{ |a| a.publised_at > 5.days.ago } There are many ways to get the functionality that your after... Many more than I know about. Which one to use is probably the tricky part. Do you use the has_many option from the previous post. It may hit the db a few more times. But if you grab the Articles as above via a find(:all) and there are big articles and plenty of them, then your memory usage may be too much. You could alleviate that by articles = Articles.find(:all, :limit => 100, :order => "published_at DESC" ) either way, if this is going to be something that you do regularly it is best, I believe, to wrap it up into a method. This will keep the logic in one place, and if you need to change it you will know exaclty where to go. Your third suggestion looks interesting, I hadn''t thought of that, have> to give that one some thought now. > > Thanks for the reply. If you have any more thoughts on what I said or > any others, don''t hesitate. I''m trying to broaden my programming skills > by learning from best practices and getting good feedback. Thanks again > Daniel. > > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > Rails mailing list > Rails@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails >-------------- next part -------------- An HTML attachment was scrubbed... URL: http://wrath.rubyonrails.org/pipermail/rails/attachments/20060617/a0bfe59e/attachment.html
You are right Daniel, what I should have been saying was instance variable. It is a per-object reference that I was thinking about. My mistake. So in that case then, would you think that it might be better design to keep the active state stored as that instance variable, therefore avoiding passing it into all the method calls, or keeping it explicitly in the method calls? Was there more to your comment as it seems to have been cut off, I think the forum may have gone down for a second. -- Posted via http://www.ruby-forum.com/.
On 6/17/06, Laz <stroke_it@hotmail.com> wrote:> > You are right Daniel, what I should have been saying was instance > variable. It is a per-object reference that I was thinking about. My > mistake. So in that case then, would you think that it might be better > design to keep the active state stored as that instance variable, > therefore avoiding passing it into all the method calls, or keeping it > explicitly in the method calls?In my opinion it is definitley good practice for an object to know it''s state. In theoretical OOP design, an object should not allow other objects access to it''s internal workings. It should provide a way for other objects to ask it what a property is set to, and also tell it to update it''s properties. Basically an object should be responsible for it''s data. There are many tutorials on OOP design on the web and I suggest that you take the time to read some of them. They are generally very good. The Pickaxe and Why''s Poignant guide are good. (Whys book is different but informative). http://www.ruby-doc.org/docs/ProgrammingRuby/ http://poignantguide.net/ruby/ Other book resources for ruby can be found at http://new.ruby-lang.org/en/documentation/book-list/ This is a very important concept in Ruby since everything is an object and basically works this way. So an instance variable regarding status should be hidden from other objects, but accessible with instance methods to get status and set status. ie I ask an article, are you active? and it tells me if it is. In rails whatever fields the db table has, the corresponding model will be endowed with getter and setter methods for each field automatically :) That means that if you have a status field in your database table, then the model will already have the methods required to access it and set it. @my_model.status # returns the status variable @my_model.status = "inactive" # to set the status To make it''s state persistant (save it to the db) @my_model.save! ActiveRecord is very nice for this. But this by itself doesn''t help you find them which I believe was the subject of your initial post. To do this you use the find method provided by the ActiveRecord class. Since you model inherits from this class, your model gets a find method too. If you have a time where you are regularly performing the same action for a given outcome, then convinience methods are good. def self.active_articles find(:all, :conditions => { :active => true } end By declaring this you can now use Article.active_articles If you decide to change the definition of what makes an article active, you don''t need to surf your code and find everywhere you did my_method_to_find_articles( "active" ) or find(:all, :conditions => { :active => true } ) which may appear many times in many files to change the logic. If in your code you aske the Article class or some parent object to get all the active articles when you want them, then you can change your logic in one place whenever required. def self.active_articles find( :all, :conditions=> { :active => true, :archive => false } end This is commonly refered to as DRY (Don''t Repeat Yourself) and makes code more readable and maintainable. I am certainly no expert on this. I am new to programming in general and it''s not my day job. I really suggest you try looking at the Wiki on www.rubyonrails.org and also try to find some blogs about this stuff. People who know it much better than I do have put a lot of time into explaining these concepts and you can learn a lot by reading their blogs. The rails blog is a good source. http://weblog.rubyonrails.org/ and they also link to good blogs to get you started. Cheers Was there more to your comment as it seems to have been cut off, I think> the forum may have gone down for a second. > > -- > Posted via http://www.ruby-forum.com/. > _______________________________________________ > Rails mailing list > Rails@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails >-------------- next part -------------- An HTML attachment was scrubbed... URL: http://wrath.rubyonrails.org/pipermail/rails/attachments/20060617/2c909f3e/attachment.html