|
»
»
»
RSpec makes me want to write better code
|
View:
New views
20 Messages
—
Rating Filter:
Alert me
|
| < Prev | 1 - 2 | Next > |
 |
RSpec makes me want to write better code
by Silpan Patel
::
Rate this Message:
Hi,
Today is a big day. I officially transitioned from manually testing by clicking around in my app, to automated testing with RSpec + Autotest. Yes RSpec made me find a few weaknesses in my app: while I was writing specs for one of my models, I discovered that I had forgotten some validations, that simply I had never tested by my super manual clicking workflow. Also, RSpec made me discover something else: my model has some custom find methods. Often over time I find myself changing the name of these custom find methods, e.g: find_all_products -> find_available_products As some of these finds are used by more than 1 controller, changing the name of the find_ often breaks code in different places at once. This was painful to manually test, and it is still a bit painful to have RSpec test, as I would also have to rename the custom find method in my specs. Based on this recent article: http://www.matthewpaulmoore.com/articles/1276-ruby-on-rails-code-quality-checklist "Each controller action only calls one model method other than an initial find or new.". Would this mean that I should change my custom find naming convention to something more general that would never have to be renamed over time such as find_for_index, find_for_create, etc? What are your thoughts on that. Is there a "design pattern" on naming custom find methods in Rails models? Because I tend to be seeing one. Also, is it clever to write specs such as: -- Product.respond_to? :find_for_index -- so that if I break the rule of my naming convention, one of my spec will quickly bark at me. Best regards, -- Posted via http://www.ruby-forum.com/. _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Thu, Sep 25, 2008 at 8:01 AM, Fernando Perez <lists@...> wrote:
Didn't find_all_products do something different from find_available_products? If so, it's not really a renaming issue, as far as I can see. You're changing behavior, therefore you change the specs. Based on this recent article: I didn't get that article (or, rather, that particular subarticle) at all. It sounded to me like some of the virulent REST advocates who want to shoe-horn every action into an HTTP verb. The controller should do what the controller needs to do. If that means displaying just available products instead of all products, it should do that "with intention," not just as the default find action. That said, scopes can and should be defined, but that's a well-understood idiom of Rails programming and can lead to abuse. Would this mean that I should change my custom find naming convention to something more general that would never have to be renamed over time That puts too much controller knowledge in the model, in my opinion. Also, is it clever to write specs such as: I don't think it really matters whether Product has a find_for_index method in and of itself. What matters is that the desired behavior, possibly including and/or implemented by calling find_for_index, is achieved. If such a naming convention was very important to the project, then a spec like that would be appropriate. However, I'd regard it as a code smell if I saw such a test. I can imagine other opinions on this subject and am eager to hear them. ///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Ashley Moran-4
::
Rate this Message:
On 25 Sep 2008, at 17:48, Mark Wilden wrote: > "Each controller action only calls one model method other than an > initial find or new.". > > I didn't get that article (or, rather, that particular subarticle) > at all. I kinda tuned out when I read, "Polymorphic associations, however, are encouraged!" I wouldn't let that guy near a CSV file of my shopping list after reading that... > It sounded to me like some of the virulent REST advocates who want > to shoe-horn every action into an HTTP verb. The controller should > do what the controller needs to do. If that means displaying just > available products instead of all products, it should do that "with > intention," not just as the default find action. Is a better rule "each controller action should contain no more than two branches"? (But then, I try to apply that to all methods, and even then, I try to push conditional code as far down as possible.) > That said, scopes can and should be defined, but that's a well- > understood idiom of Rails programming and can lead to abuse. > > Would this mean that I should change my custom find naming > convention to > something more general that would never have to be renamed over time > such as find_for_index, find_for_create, etc? > > That puts too much controller knowledge in the model, in my opinion. I'd try identify the problem that is being solved in the controller. *Why* does ProductController#index need that certain subset of the data? Maybe the model method should be something like: * Product.top_selling * Product.not_selling * Product.needs_updating Does this better express the intent? > What matters is that the desired behavior, possibly including and/or > implemented by calling find_for_index, is achieved. If such a naming > convention was very important to the project, then a spec like that > would be appropriate. However, I'd regard it as a code smell if I > saw such a test. I agree (I think). Conventions decrease the mental effort to make something work at the risk of reduced insight. Certainly if you are automatically checking that the code is following conventions, that suggests the convention is artificial and unnatural. You could end up trying to map all the business problems onto a very crude technical interface. Ashley -- http://www.patchspace.co.uk/ http://aviewfromafar.net/ _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Matt Wynne-2
::
Rate this Message:
On 26 Sep 2008, at 12:31, Ashley Moran wrote:
> > On 25 Sep 2008, at 17:48, Mark Wilden wrote: > >> "Each controller action only calls one model method other than an >> initial find or new.". >> >> I didn't get that article[1] (or, rather, that particular >> subarticle) at all. > > I kinda tuned out when I read, "Polymorphic associations, however, > are encouraged!" Would you mind elaborating on why you don't like these? I'm pretty new to rails (but not programming generally) and rather naive about such things! Also why is the article so down on STI? What are the drawbacks? What do people use instead? [1]http://www.matthewpaulmoore.com/articles/1276-ruby-on-rails-code- quality-checklist cheers, Matt ---- http://blog.mattwynne.net http://songkick.com In case you wondered: The opinions expressed in this email are my own and do not necessarily reflect the views of any former, current or future employers of mine. _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Fri, Sep 26, 2008 at 4:49 AM, Matt Wynne <matt@...> wrote:
I think the guy is really just down on inheritance itself, which is not an unusual nor even entirely unjustified attitude. Ruby has far less need to use inheritance because of the ability to mix in modules. And using it, particularly beyond two levels deep, can most definitely lead to nightmarish code, where changing one line in a parent class can make you spend an hour (or a day) trying to figure out how to handle the repercussions to every child class.
But I do believe that where a true "is-a" relationship exists, inheritance is often the best approach. I'm working on a program right now that has a Task class and an Appointment subclass. An Appointment (in this context) is simply a Task that can only be performed on one day. Otherwise it's exactly like a Task (again, in the context of my program). That's an "is-a" relationship that inheritance was designed for, and it's working quite well. And, as an agile practitioner, if I find that this relationship changes I would throw out this class hierarchy.
STI is just a way to map inheritance to the database. I used it before I knew what it was called. If you do use inheritance with models, I think it's the best Rails way to persist the data. Hmmm. Just realized that all this has nothing to do with RSpec.... ///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Fri, Sep 26, 2008 at 4:31 AM, Ashley Moran <ashley.moran@...> wrote:
On an OOP mailing list, a guy asserted that there shouldn't be two for-loops in the same method. It sounded ridiculous to me at the time, but after ten years of mulling it over, I believe he was right. :)
///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Ashley Moran-4
::
Rate this Message:
On 26 Sep 2008, at 12:49, Matt Wynne wrote: > Would you mind elaborating on why you don't like these? I'm pretty > new to rails (but not programming generally) and rather naive about > such things! It's quite hard to explain briefly, but basically it makes the predicate (interpretation of the table) extremely difficult. A normal table like cars --------- reg_plate owner_id purchase_date Could be interpreted as "The car with reg plate <reg_plate> was bought by the person with ID <owner_id> on <purchase_date>" (ok, they might have sold it since, but...) However, from the Rails wiki[1]: addresses --------- street city country addressable_id addressable_type # <- is a string How do you interpret this? The relationship is fundamentally different depending on the value of addressable_type, and is much harder to enforce as an integrity constraint*. There's another angle you can take, based on data types - you can define a type PERSON_ID for the attribute 'owner_id' (and re-use it in the 'departments' table as 'manager_id'), but the type of 'addressable_id' depends on the value of 'addressable_type'. How can a value not know its own type? Easier solution: people --------- name delivery_address_id billing_address_id "The person called <name> wants items delivered to the address with ID <delivery_address_id> and invoices delivered to the address with ID <billing_address_id>". > Also why is the article so down on STI? What are the drawbacks? What > do people use instead? > > [1]http://www.matthewpaulmoore.com/articles/1276-ruby-on-rails-code-quality-checklist I'll reply to Mark... TBC Ashley [1] http://wiki.rubyonrails.org/rails/pages/UnderstandingPolymorphicAssociations * pah, who needs their data integrity protected anyway? -- http://www.patchspace.co.uk/ http://aviewfromafar.net/ _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Ashley Moran-4
::
Rate this Message:
On 26 Sep 2008, at 15:16, Mark Wilden wrote: > Also why is the article so down on STI? What are the drawbacks? What > do people use instead? One downside to STI is it forces you to leave NULL columns for attributes that don't exist in all models. This is also really bad for integrity. I once started a CTI[1] extension to AR[2][3]. A lot of users were interested in it, but I got no response from the core team. There's talk on doing it in DataMapper too. If I get some free time I might look at it, but it's a fairly complex bit of ORM work. (I did want to write an ORM but I spend way too much time doing paid work...) > I think the guy is really just down on inheritance itself, which is > not an unusual nor even entirely unjustified attitude. Ruby has far > less need to use inheritance because of the ability to mix in modules. > > ... > > But I do believe that where a true "is-a" relationship exists, > inheritance is often the best approach. I'm working on a program > right now that has a Task class and an Appointment subclass. An > Appointment (in this context) is simply a Task that can only be > performed on one day. Otherwise it's exactly like a Task (again, in > the context of my program). That's an "is-a" relationship that > inheritance was designed for, and it's working quite well. And, as > an agile practitioner, if I find that this relationship changes I > would throw out this class hierarchy. That sounds like a perfect example. I assume any code that works for "Task" will happily process an appointment. > STI is just a way to map inheritance to the database. I used it > before I knew what it was called. If you do use inheritance with > models, I think it's the best Rails way to persist the data. Not the best possible way (which is CTI), but yeah, the best way you can do it with Rails (ActiveRecord). It's still a data modelling/ integrity headache, but not on the same level as polymorphic associations. Ashley [1] http://martinfowler.com/eaaCatalog/classTableInheritance.html [2] http://dev.rubyonrails.org/ticket/6566 [3] http://aviewfromafar.net/2008/1/19/class-table-inheritance-in-activerecord [4] http://wm.lighthouseapp.com/projects/4819/tickets/53-class-table-inheritance ([3] was started on my old employer's blog, but they never maintained it after I left) -- http://www.patchspace.co.uk/ http://aviewfromafar.net/ _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Fri, Sep 26, 2008 at 8:28 AM, Ashley Moran <ashley.moran@...> wrote:
I think all of your comments make sense, but I did just want to call out that "the Rails way" is not typically concerned with this sort of integrity at the database level. It's handled in the model.
Whether that's a good or bad thing can be debated, but it does explain why STI's drawbacks don't outweigh its strengths (primarily speed, compared to CTI, because of the lack of joins) in the minds of true Rails believers.
///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Ashley Moran-4
::
Rate this Message:
On 26 Sep 2008, at 17:28, Mark Wilden wrote: > I think all of your comments make sense, but I did just want to call > out that "the Rails way" is not typically concerned with this sort > of integrity at the database level. It's handled in the model. Ah ok, I wasn't sure if your comment was intended to be neutral. You're right, the Rails way is not concerned with DB-level integrity. > Whether that's a good or bad thing can be debated, but it does > explain why STI's drawbacks don't outweigh its strengths (primarily > speed, compared to CTI, because of the lack of joins) in the minds > of true Rails believers. I've avoided STI unless the models share all the attributes (and therefore you . But it can be used acceptably at a Ruby level (be sure to spec what attributes your classes have if you're scared of pollution!) - it just means more hassle if you work in the DB directly. Ashley -- http://www.patchspace.co.uk/ http://aviewfromafar.net/ _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Fri, Sep 26, 2008 at 9:47 AM, Ashley Moran <ashley.moran@...> wrote:
As part of the TDD process, I spec all attributes, but this doesn't seem universal. Is this a misconception? Do people actually make sure that all columns exist and can be written to and read from?
///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Scott Taylor-6
::
Rate this Message:
On Sep 26, 2008, at 1:18 PM, Mark Wilden wrote:
I usually end up doing something like this: columns = [:email, :message] columns.each do |column| it "should have a reader and writer for the column #{column}" do @invite.should respond_to(column) @invite.should respond_to("#{column}=") end end Scott _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Fri, Sep 26, 2008 at 10:24 AM, Scott Taylor <mailing_lists@...> wrote:
That looks like a really good way to do this - thanks! Beyond that, as they say, you're testing ActiveRecord.
///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Pat Maddox
::
Rate this Message:
"Mark Wilden" <mark@...> writes:
> On Fri, Sep 26, 2008 at 4:49 AM, Matt Wynne <span dir="ltr"><mailto:matt@...></span> wrote: > > Also why is the article so down on STI? What are the drawbacks? What > do people use instead?I think the guy is really just down on > inheritance itself, which is not an unusual nor even entirely > unjustified attitude. Ruby has far less need to use inheritance > because of the ability to mix in modules. And using it, particularly > beyond two levels deep, can most definitely lead to nightmarish code, > where changing one line in a parent class can make you spend an hour > (or a day) trying to figure out how to handle the repercussions to > every child class. But I do believe that where a true "is-a" > relationship exists, inheritance is often the best approach. I'm > working on a program right now that has a Task class and an > Appointment subclass. An Appointment (in this context) is simply a > Task that can only be performed on one day. Otherwise it's exactly > like a Task (again, in the context of my program). That's an > "is-a" relationship that inheritance was designed for, and it's > working quite well. And, as an agile practitioner, if I find that this > relationship changes I would throw out this class hierarchy. STI is > just a way to map inheritance to the database. I used it before I knew > what it was called. If you do use inheritance with models, I think > it's the best Rails way to persist the data. Hmmm. Just realized > that all this has nothing to do with RSpec....///ark > _______________________________________________ rspec-users mailing > list rspec-users@... > http://rubyforge.org/mailman/listinfo/rspec-users fwiw, my inclination would still be to model this with composition rather than inheritance :) An Appointment in the simplest case is just a DateTime, but you can imagine it having a Location, Participants, and of course a Task. If it can have a Task, it can probably have multiple Tasks. And you can imagine Task evolving independently of Appointment, for example a composite Task, or tasks requiring follow-up tasks under certain conditions. You can say YAGNI of course, but I think by splitting those out entirely, you gain a more flexible model without introducing complexity, and you avoid the technical stickiness that accompanies STI in Rails. And I think it's perfectly acceptable to talk about OOD because design is one of the key benefits of BDD :) Pat _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Mark Wilden
::
Rate this Message:
On Fri, Sep 26, 2008 at 11:46 AM, Pat Maddox <pergesu@...> wrote: "Mark Wilden" <mark@...> writes:
fwiw, my inclination would still be to model this with composition It could have all those things, but it doesn't. :) The program is a Getting Things Done-type task manager (called giterdone). It couldn't care less who the appointment is with. It just knows that an Appointment is something that needs to "get done." The appointment's location is handled by a GTD context, just like any other Task. The only difference is that this kind of Task can only be done on a particular day. If you don't giterdone on that day, it falls off the schedule.
If it can have a Task, it can probably have multiple Tasks. And you can imagine Task evolving independently of Appointment, A Project is another kind of Task - a sibling to Appointment - and can have multiple Tasks (including Appointments). This again is in line with the GTD philosophy, which draws a clear distinction between things with "next actions" and things without (like a project).
You can say YAGNI of course, but I think by Well, I do say YAGNI. Or at least YPAGNI. And again, if this turns out to be incorrect, I'll change it. I really do believe in trying the simplest thing that could possibly work. I've used STI on another side project of mine (a magic trick database, called abracadata), and it actually did work pretty damn perfectly. But if it turns out to be the wrong decision, I'll embrace that change.
However, I do admit that I'm biased in favor of inheritance where it applies - I've been using it since 1989, after all. Prior to that time, I was implementing it in C before I knew what it was called. I really like the idea of "this thing is like that thing except for one little difference."
On the other hand, I have been bitten by using inheritance where composition would have been a better design. I think the last time that happened was in 1991. :) And I think it's perfectly acceptable to talk about OOD because design
is one of the key benefits of BDD :) Coolness. ///ark _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by Ashley Moran-4
::
Rate this Message:
On Sep 26, 2008, at 6:18 pm, Mark Wilden wrote: > As part of the TDD process, I spec all attributes, but this doesn't > seem universal. Is this a misconception? Do people actually make > sure that all columns exist and can be written to and read from? What I meant by this was that say you have classes... Animal {weight, height, dob} Pet < Animal {name} ZooAnimal < Animal {cage_id} then, using ActiveRecord, all three will have attributes {weight, height, dob, name, cage_id}. If you don't want your pets in a cage, it's worth preventing that and writing specs for it. That's what I was getting at, really - the extra data corruption issues. Ashley -- http://www.patchspace.co.uk/ http://aviewfromafar.net/ _______________________________________________ rspec-users mailing list rspec-users@... http://rubyforge.org/mailman/listinfo/rspec-users |
|
|
Re: RSpec makes me want to write better code
by David Chelimsky-2
::
Rate this Message:
|
