1

I have a method in my CLI app which is giving me an error.

The method is:

def self.deal_page(input, product_url)
    self.open_deal_page(input)
    deal = {}
    html = open(@product_url)
    doc = Nokogiri::HTML(html)
    data = doc.text.strip
    deal[:name] = doc.css("h1").text.strip
    deal[:discription] = doc.css(".textDescription").text.strip
    @purchase_link = nil
    @purchase_link= doc.at_css("div.detailLeftColumn a.success").attr("href")
        if @purchase_link.nil?
         deal[:purchase] = @product_url
       else
         deal[:purchase] = @purchase_link
       end
     deal
  end

and the error is:

/home/himachhag-45739/code/popular-deals-from-slickdeals.net-cli/lib/popular_deals/newdeals.rb:54:in `deal_page': undefined method `attr' for nil:NilClass (NoMethodError) 
        from /home/himachhag-45739/code/popular-deals-from-slickdeals.net-cli/lib/popular_deals/cli.rb:70:in `disply_deal'                                                 
        from /home/himachhag-45739/code/popular-deals-from-slickdeals.net-cli/lib/popular_deals/cli.rb:49:in `menu'                                                        
        from /home/himachhag-45739/code/popular-deals-from-slickdeals.net-cli/lib/popular_deals/cli.rb:9:in `call'                                                         
        from /home/himachhag-45739/code/popular-deals-from-slickdeals.net-cli/bin/popular-deals:10:in `<top (required)>'                                                   
        from /usr/local/rvm/gems/ruby-2.3.1/bin/popular-deals:22:in `load'                                                                                                 
        from /usr/local/rvm/gems/ruby-2.3.1/bin/popular-deals:22:in `<main>'                                                                                               
        from /usr/local/rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in `eval'                                                                                         
        from /usr/local/rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in `<main>'

I tried xpath, at_css, unless, if..else, but doesn't help. Also, I don't get this error everytime, but I would like to get rid of it.

the Tin Man
  • 158,662
  • 42
  • 215
  • 303
Hima Chhag
  • 401
  • 7
  • 22
  • 1
    When facing `nil` problems like this it's important to step back one operation and see what failed to engage. It looks like `doc.at_css(...)` didn't find anything. Another thing to note is to try and keep your indentation consistent. That `if` clause is shoved in there like it's slipped out of place. – tadman Apr 06 '17 at 16:56
  • @tadman Thank you so much for your suggestion. I agree with you that It looks like doc.at_css(...) didn't find anything. But if I try again to look for the same deal, It does give me output! which I don't understand. I will be more careful about indentation too. – Hima Chhag Apr 06 '17 at 16:59
  • Welcome to SO. Please read "[mcve]" and the linked page. You need to provide code that allows us to confirm the problem. Currently we can't do that because you didn't tell us how to call your method. – the Tin Man Apr 06 '17 at 17:53

3 Answers3

4

One way to address this problem is to be a bit paranoid:

@purchase_link = doc.at_css("div.detailLeftColumn a.success").try(:attr, "href")

deal[:purchase] = @purchase_link || @product_url

A couple of things to keep in mind here. In Ruby only nil and false are logically false, so it's very rare you need to specifically test nil? on things. The only case that's necessary is when you want to distinguish between nil and false, which as you can imagine is not very often.

So in this case either you hit something with at_css or you didn't, in which case the try call doesn't do anything. If it finds something the try call carries on for one more call. Then you can do an assignment with a simple || (or) operator to pick them in priority.

Another thing is since this code is inside a class method, casually using instance variables can be trouble. If things like purchase_link are only used within this method, remove the @ which makes them persistent.

Another thing to be careful about is how your method is defined as:

def self.deal_page(input, product_url)

That declares an argument product_url, but inside:

html = open(@product_url)

This references the class instance variable @product_url which is not the same. You may be calling open with the wrong variable here.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • 3
    Note that `.try()` is a [Rails](https://apidock.com/rails/v3.2.1/Object/try) method, not a core Ruby method. The [safe navigation operator](http://stackoverflow.com/questions/36812647/what-does-ampersand-dot-mean-in-ruby) would be the Ruby equivalent with `doc.at_css()&.attr()`. – anothermh Apr 06 '17 at 17:51
  • 2.3 introduced the safe navigation operator which is actually a lot more coherent than `try`. I forgot that was Rails specific. – tadman Apr 06 '17 at 17:53
  • 1
    @tadman This solution works! I really appreciate your time and suggestions. – Hima Chhag Apr 06 '17 at 18:13
  • 1
    @anothermh Thanks a lot. I was unfamiliar with safe navigation operator. I will keep this in my mind. – Hima Chhag Apr 06 '17 at 18:14
0

As your stack trace shows this line is causing the error:

@purchase_link= doc.at_css("div.detailLeftColumn a.success").attr("href")

attr method can not be called on nil. See if the element exists in the HTML.

You can debug this by printing the value of 

doc.at_css("div.detailLeftColumn a.success")

For more information you can refer to http://www.nokogiri.org/tutorials/.

the Tin Man
  • 158,662
  • 42
  • 215
  • 303
yatindra rao
  • 107
  • 7
0

Here's where I think the problem is:

def self.deal_page(input, product_url)
  ...
  html = open(@product_url)

You're using product_url as a parameter, but trying to open @product_url.

open will raise an error if @product_url is empty or nil, so @product_url must be predefined somewhere, but obviously not in this method. I suspect it's not the page you think, and, as a result the selector fails.

You have other problems in your code:

deal[:name] = doc.css("h1").text.strip
deal[:discription] = doc.css(".textDescription").text.strip

You're using css, which returns a NodeSet

doc.css('h1').class # => Nokogiri::XML::NodeSet

then text which concatenates all text in the NodeSet, which is almost always not what you want to do. Consider this:

require 'nokogiri'

doc = Nokogiri::HTML(DATA.read)
doc.css('h1').text # => "foobar"
doc.css('h1').map(&:text) # => ["foo", "bar"]

__END__
<html>
  <body>
    <h1>foo</h1>
    <h2>blah</h2>
    <h1>bar</h1>
    <h2>blah</h2>
  </body>
</html>

doc.css('h1').text concatenated "foo" and "bar" resulting in "foobar". Once you have that occur it's extremely difficult to unravel the chaos caused by the concatenation.

Instead, you should use doc.css('h1').map(&:text), except in that rare situation where you know that the text actually needs to be concatenated. I've only run into that situation... oh... never times.

the Tin Man
  • 158,662
  • 42
  • 215
  • 303