0

I am pretty new to programming and need some help/feedback on my code. My goal is to scrape my data, which is working fine, and then display that data to my user in a numbered list. I am simply having difficulty displaying this data. I do not get any errors back my program simply skips my method altogether. Thanks in advance for any help/feedback!

class BestPlaces::Places
  attr_accessor :name, :population, :places
    @@places = []

  def self.list_places
    # puts "this is inside list places"
    self.scrape_places
  end

      def self.scrape_places
        doc = Nokogiri::HTML(open("https://nomadlist.com/best-cities-to-live"))
            places = doc.search("div.text h2.itemName").text
            rank = doc.search("div.rank").text

            places.collect{|e| e.text.strip}
              puts "you are now in title"
              @@places << self.scrape_places
              puts "#{rank}. #{places}"
            end
          end
        end

CLI Page:
class BestPlaces::CLI

  def list_places
    puts "Welcome to the best places on Earth!"
    puts @places = BestPlaces::Places.list_places
  end

  def call
    list_places
    menu
    goodbye
  end
end
schall
  • 29
  • 6
  • 1
    How do you run your code? – spickermann May 27 '17 at 19:42
  • Locally, inside my bin folder just a CLI program – schall May 27 '17 at 19:52
  • You have to invoke your method, ie `BestPlaces::Places.list_places` (i think that's your entry point). Put that at the bottom of your file. It looks like though you have and endless loop at line `@@places << self.scape_places`. – Anthony May 27 '17 at 20:03
  • Yes, list_places is my entry point. Thanks I will play with the loop. I do want to push the data into an array though. – schall May 27 '17 at 20:17
  • Also, 1) check your `end`s, it seems that you have one _hanging around_ in `BestPlaces::Places` class; 2) `places.collect` will fail, since `text` in `doc.search("div.text h2.itemName").text` will return a `String` object. – Gerry May 28 '17 at 00:37

1 Answers1

0

There are a few things that could be addressed in this code, but let's first see a reworking: 

require 'nokogiri'
require 'open-uri'

module BestPlaces

  class Places
    attr_accessor :name, :population, :places

    def initialize
      @places = []
    end

    def scrape_places
      doc = Nokogiri::HTML(open("https://nomadlist.com/best-cities-to-live"))
      places = doc.search("div.text h2.itemName")
      ranks = doc.search("div.rank")
      places.each{|e| @places << e.text.strip}
      puts "you are now in title"
      @places.each do |place|
        i = @places.index(place)
        puts "#{ranks[i].text}. #{place}"
      end
   end

 end

 class CLI

   def list_places
     puts "Welcome to the best places on Earth!"
     BestPlaces::Places.scrape_places
   end

   def call
     list_places
     menu
     goodbye
   end

 end

end

You have what looks to be an incomplete Module/Class setup. One can call the above like so: 

bp = BestPlaces::Places.new
bp.scrape_places

The @@places variable was unnecessary, we can use @places instead to hold values that need to be accessed within the Places class. Also, nokogiri returns a string object when using the .text method on search results, which means you cannot iterate over them like an array. I hope this helps.

Andrew Long
  • 1
  • 1
  • 2
  • Thank you everyone! I really appreciate the feedback! – schall May 28 '17 at 21:53
  • would this be the reason why my push method is unidentified by ruby? My program is not recognizing my '<<' as a push method. – schall May 29 '17 at 23:46
  • If you're referring to the push method you had in self.scrape_places, you were trying to push a value that wasn't there, e.g. the method didn't have a return statement to actually return something. You also might want to look [here](https://stackoverflow.com/questions/1023146/is-it-good-style-to-explicitly-return-in-ruby) and [here](https://stackoverflow.com/questions/10569529/ruby-difference-between-array-and-arraypush) for info on return statements and usage of the '<<' method. – Andrew Long May 30 '17 at 13:29