Skip to content

most fun assignment yet#68

Open
laconiccrafts wants to merge 4 commits intovikingeducation:masterfrom
laconiccrafts:master
Open

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

Conversation

@laconiccrafts
Copy link

No description provided.

Copy link

@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.

require "csv"

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

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.

@agent = $agent
end

def parse_date(s)

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

end
end

def to_csv(path)

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

end
end

def parse_options(opts)

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

"q-#{query}-l-#{location}-radius-#{radius}-startPage-#{startpage}-jobs"
end

def parse_jobs(jobs)

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

(posted.day >= date.day) && (posted.month >= date.month)
end
return acc if jobs.empty?
startpage = opts[:startpage]

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
Author

Choose a reason for hiding this comment

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

why?

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