Skip to content
This repository has been archived by the owner on May 1, 2018. It is now read-only.

hw1\hw2 #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

hw1\hw2 #36

wants to merge 2 commits into from

Conversation

A-Lada
Copy link

@A-Lada A-Lada commented Nov 15, 2016

No description provided.

denysxftr
denysxftr reviewed Nov 22, 2016
View reviewed changes
Copy link
Owner

@denysxftr denysxftr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure that i understand purpose of @reserved_words but it looks like your code is hardcoded a bit

hw2/lib/router.rb
def call(env)
res = detect_route(env['REQUEST_METHOD'], env['REQUEST_PATH'])
if (!res || res.empty?)
then
Copy link
Owner

Choose a reason for hiding this comment

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

you don't need then

hw2/lib/router.rb
then
[404, {}, ['page not found']]
else
env['REQUEST_PATH'] = res
Copy link
Owner

Choose a reason for hiding this comment

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

why do you assign res to env['REQUEST_PATH'] ?
you can just @routes[env['REQUEST_METHOD']][res].call(env)

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

@denysxftr denysxftr denysxftr left review comments

Assignees
No one assigned
Labels
checked_1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@A-Lada @denysxftr