Skip to content

Added Behdad Analui's solution#63

Open
banalui wants to merge 5 commits intovikingeducation:masterfrom
banalui:master
Open

Added Behdad Analui's solution#63
banalui wants to merge 5 commits intovikingeducation:masterfrom
banalui:master

Conversation

@banalui
Copy link
Copy Markdown

@banalui banalui commented Jan 9, 2017

No description provided.

Copy link
Copy Markdown

@remis1889 remis1889 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Too many classes than necessary. Should be refactored.

Comment thread dice_web_scraper.rb
@all_jobs = []
end

def extract_job_info(search_result)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the methods except scrape and save should be private

Comment thread page_formatter.rb

class PageFormatter

def self.get_page_from(url)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is not necessary. get_page_from can be a method in DiceWebScraper

Comment thread dice_web_scraper.rb
extract_job_info cur_page
end

def filter_result(search_result)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method can be removed since all it does is call another method. You may as well call filter_segment directly from scrape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants