Skip to content

most fun assignment yet#68

Open
laconiccrafts wants to merge 4 commits into
vikingeducation:masterfrom
laconiccrafts:master
Open

most fun assignment yet#68
laconiccrafts wants to merge 4 commits into
vikingeducation:masterfrom
laconiccrafts:master

Conversation

@laconiccrafts
Copy link
Copy Markdown

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.

Comment thread lib/scraper.rb
require "csv"

$agent = Mechanize.new{ |agent| agent.user_agent_alias = "Mac Safari" }
$agent.history_added = Proc.new { sleep 0.5 }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These 2 lines can also be included in the initialize method of class Scraper.

Comment thread lib/scraper.rb
@agent = $agent
end

def parse_date(s)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should be a private method

Comment thread lib/scraper.rb
end
end

def to_csv(path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should be a private method

Comment thread lib/scraper.rb
end
end

def parse_options(opts)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should be a private method

Comment thread lib/scraper.rb
"q-#{query}-l-#{location}-radius-#{radius}-startPage-#{startpage}-jobs"
end

def parse_jobs(jobs)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should be a private method

Comment thread lib/scraper.rb
(posted.day >= date.day) && (posted.month >= date.month)
end
return acc if jobs.empty?
startpage = opts[:startpage]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

an extra variable, startpage is not necessary

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

why?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you can directly use opts[:startpage] intead of startpage

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