Well, it seems like this is all the feedback there is likely to be, and the majority wants things the way they are. My current git fork is changed to reflect this. Mostly. I left the bang-method in - this is useful to me to replace the rather frequent case I run into where I do
div=ie.div(:text, some_text) unless div.exists? raise "the specified div doesn't exist" end I check immediately, because raising the error later on when I try to use the div (the standard method for watir's unit tests), at some other point in my code, would make debugging a nightmare - having to trace back through to where the div was instantiated, which is sometimes very far-removed from where it is used. I want things to fail immediately, so I use the bang-method. (on reflection I guess I should just be doing div.assert_exists - though that's private, so div.send(:assert_exists) - ah well, I inherited the code I work on, and that is among many things that I'd have done differently. same idea in any case.) I added another method to perform the behavior that I had container methods such as Container#div doing - returning an Div when the element exists; returning nil on nonexistence - to happen in new method, Container#div? This is a bit of a break from ruby convention where question methods return true/false (though even ruby core doesn't strictly adhere to that), but I think it is appropriate, as the point is to use it in conditionals where I would otherwise have to separate retrieving the div and checking its existence. So, to summarize: • container.div(how, what) => Div, always, whether exists or not • container.div!(how, what) => Div or raise UnknownObjectException • container.div?(how, what) => Div or nil It seems like perhaps an excessive number of methods for basically the same operation. But, the first one has basically no place in my code, as I always want to immediately error or check existence, never just pass a nonexistent element along to be used. But, that is what this community is used to and wants, so it stays. and the latter two are too useful for me to remove from my fork - and hopefully the community will find them useful as well? -Ethan On Sun, Oct 11, 2009 at 00:06, Bret Pettichord <[email protected]> wrote: > That sounds right. > > > On Sat, Oct 10, 2009 at 4:41 PM, <[email protected]> wrote: > >> Initially as a user I just expected it to work this way... >> >> unless browser.div(:id, 'body').button(:id, 'save') ... >> >> But it didn't take long through the examples to learn about the exists? >> method. I primarily rely on Waiter to do my syncing between pages, often >> just looking for text rather than page elements >> >> Watir::Waiter.wait_until{ browser.html.include?("domloaded") } >> >> My understanding is that I could change that block to be >> >> Watir::Waiter.wait_until{ browser.div(:id, 'body').button(:id, >> 'save').exists? } >> >> and would achieve the same goal no? >> >> >> >> On 11/10/2009 7:50am, Bret Pettichord <[email protected]> wrote: >> > Ethan, >> > >> > Thanks for your thoughtful comments. >> > >> > Originally, I wanted exists? to work this way: >> > >> > browser.div(:id, 'body').button(:id, 'save').exists? >> > >> > would return false if either the div or the button didn't exist. We've >> never implemented it to work this way, but I think it would still be a good >> idea and more in line with the expectations and needs of our typical users. >> I would welcome a patch that made Watir work this way. Your proposal would >> not make this possible. >> > >> > >> > As you note, the current implementation does provide better error >> reporting that what we'd get with your proposal. Very early versions of >> Watir, in fact, worked as you suggest -- it is the "natural" and easiest way >> of implementing it. But we added support for the current, less-natural >> behavior because it allowed us to give more helpful error messages to our >> users. >> > >> > >> > Bret >> > >> > On Sat, Oct 10, 2009 at 3:12 PM, Ethan [email protected]> wrote: >> > >> > I'm breaking this off into a separate thread as it is the most major >> change I made, and the most subject to discussion. (Same Ethan here, but at >> home instead of at work.) >> > >> > >> > >> > >> > On Sat, Oct 10, 2009 at 3:22 AM, Ethan wrote: >> > >> > >> > >> > >> > >> > The most major user-facing change is that I have made Container methods >> named after elements (such as #text_field, #div) return nil if the specified >> element does not exist. The previous practice of returning an element that >> did not exist, and then calling exists? to check if it exists, seemed >> unproductive to me; returning nil rather than a non-existent (and therefore >> pretty useless) object seems more natural. >> > >> > >> > >> > >> > I have also added corresponding bang-methods (#text_field!, #div!) that >> error with Watir::Exception::UnknownObjectException if the specified element >> does not exist. These are useful when you expect that an element will be >> where you are looking, and it not being there is, well, exceptional. This >> seems more logical than raising UnknownObjectException when trying to use >> any method on a nonexistent object - error when instantiating an element, if >> it is expected to exist, rather than when using it later on. >> > >> > >> > >> > >> > >> > Jari Bakken wrote: >> > This is a mistake in my opinion. The returned object is not useless - it >> contains information about how to locate the element on the page. I don't >> see any real benefits to changing this, AND it breaks >> backwards compatibility. There are some benefits to the way it's >> currently working though, especially when writing abstractions around >> Watir objects. Consider this example: >> > >> > >> > >> > >> > http://gist.github.com/206839 >> > >> > >> > >> > >> > One of the benefits of Watir over Selenium++ IMO have always been >> that it's more object-oriented - and the 'lazy locate' is definitely part of >> what makes the above example easy to write. >> > >> > >> > >> > >> > Ethan wrote: >> > >> > >> > >> > >> > >> > I have also added corresponding bang-methods (#text_field!, #div!) that >> error with Watir::Exception::UnknownObjectException if the specified element >> does not exist. These are useful when you expect that an element will be >> where you are looking, and it not being there is, well, exceptional. This >> seems more logical than raising UnknownObjectException when trying to use >> any method on a nonexistent object - error when instantiating an element, if >> it is expected to exist, rather than when using it later on. >> > >> > >> > >> > >> > >> > Jari Bakken wrote: >> > >> > >> > This goes against API design princible of making "common things easy and >> rare things possible" - in most cases you want the exception raised, so that >> should be the default syntax. I would have to add exclamation marks all over >> my code. Also this is tied to the previous point - i.e. these methods are >> only needed if the nil change is accepted (which I'm against). >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > For me at least - for my usage of Watir - it is not the case that it >> goes against the principle of making common things easiest. In my case I am >> testing a page and not certain of whether an element is on the page where I >> am looking, and I always have to check .exists? before I continue. just >> checking the return value of the container method would be more >> straightforward. >> > >> > >> > >> > This is in contrast with how the Watir unit tests tend to operate, where >> they interact with static pages, and it is expected that elements are where >> they are. My own usage and the unit tests are the only two usages of Watir I >> am particularly aware of; I would be interested to know if other people's >> usage is more similar to one or the other of those. >> > >> > >> > >> > >> > >> > The change to returning nil is only any different when an object doesn't >> exist. In that case, it's still returning a value that does not correspond >> to any element on the page: my way, it's nil; the current way, it's an >> element that can't be located and will raise exception if used. >> > >> > >> > >> > >> > >> > >> > >> > the returned object is not useless - it contains information about how >> to locate the element on the page. >> > >> > >> > >> > >> > What use is this information? The element is not on the page, so knowing >> how to locate it doesn't really do much good. >> > >> > >> > >> > The exception to that (the only one I can think of) is where AJAX loads >> elements onto a page - this is a discussion that I had with a coworker >> regarding this change. The example was elements appearing in the dom due to >> ajax calls, so say we do: >> > >> > >> > >> > >> > >> > javascript_button=browser.button(:id, 'foo') # say this button causes a >> table to pop into existence when clickedjavascript_button.click >> >> > >> > >> > table=browser.table :id, 'newly_existing_table' >> > >> > >> > >> > >> > that will fail if the #click calls to something that returns before >> 'newly_existing_table' pops into existence, as with an ajax call or a >> setTimeout. >> > >> > >> > to make it work the old way, it would be something like: >> > >> > >> > >> > >> > >> > >> > table=browser.table :id, 'newly_existing_table'until table.exists? >> > >> > >> > sleep 1end >> > >> > >> > >> > >> > my way, it would be something like: >> > >> > >> > >> > >> > >> > until table=browser.table :id, 'newly_existing_table' sleep 1 >> > >> > >> > end >> > >> > >> > Still, the latter seems more natural to me. >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > it breaks backwards compatibility >> > >> > >> > >> > That is a major sticking point, it does break compatibility when >> elements are not found. >> > One possibility is defining nil.exists? => false. My project does this >> (though I would prefer to remove reliance on it), and perhaps Watir >> including this would improve transitioning (hypothetically, if this ends up >> being the way things are done). >> > >> > >> > >> > But the exception raised on usage is different. Calling, say, >> browser.text_field(:id, 'nonexistent').click raises "NoMethodError: >> undefined method `click' for nil:NilClass"; the current way it >> raises UnknownObjectException. >> > >> > >> > >> > >> > >> > It is indeed a major API change that would break things, but I think it >> ends up being more natural and is worth the transition. >> > >> > >> > >> > >> > >> > >> > >> > >> > Consider this example: >> > >> > >> > >> > http://gist.github.com/206839 >> > >> > >> > >> > >> > >> > >> > >> > I think that my way actually improves this example. You don't need the >> loading? method at all; it is implicit in the return value of loading_div. >> #save can simply use while !loading_div. >> > >> > >> > >> > You'd probably want to change browser.button(:id, 'save') >> to browser.button!(:id, 'save'), since you expect it to exist (since it gets >> clicked without checking .exists?). That runs into the issue you mention of >> adding !s all over the code. >> > >> > >> > >> > >> > >> > I think of the ! as your code asserting that it expects a thing to >> exist, and expects the error if it doesn't. Before, it was implied that code >> always expected an element to exist, and had to go out of its way calling >> .exists? if it wasn't sure. That just wasn't consistent with how I used >> Watir. But maybe my usage is not the norm. >> > >> > >> > >> > >> > >> > Ethan >> > >> > >> > >> > >> > >> > >> > >> > >> > _______________________________________________ >> > >> > Wtr-development mailing list >> > >> > [email protected] >> > >> > http://rubyforge.org/mailman/listinfo/wtr-development >> > >> > >> > >> > -- >> > Bret Pettichord >> > Lead Developer, Watir, www.watir.com >> > >> > >> > Blog, www.io.com/~wazmo/blog <http://www.io.com/%7Ewazmo/blog> >> > Twitter, www.twitter.com/bpettichord >> > >> > >> >> _______________________________________________ >> Wtr-development mailing list >> [email protected] >> http://rubyforge.org/mailman/listinfo/wtr-development >> > > > > -- > Bret Pettichord > Lead Developer, Watir, www.watir.com > > Blog, www.io.com/~wazmo/blog > Twitter, www.twitter.com/bpettichord > > > _______________________________________________ > Wtr-development mailing list > [email protected] > http://rubyforge.org/mailman/listinfo/wtr-development >
_______________________________________________ Wtr-development mailing list [email protected] http://rubyforge.org/mailman/listinfo/wtr-development
