Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback #1

Open
sumeetjain opened this issue Feb 13, 2015 · 1 comment
Open

Feedback #1

sumeetjain opened this issue Feb 13, 2015 · 1 comment

Comments

@sumeetjain
Copy link

Can you make this method work with an options hash?

https://github.com/uanwar88/2015-02-12-warehouse-web/blob/master/main.rb#L26


Best practice is to set variables when possible in the route handler and then use them in views.

E.g. @categories = Category.list_all and then using @categories in https://github.com/uanwar88/2015-02-12-warehouse-web/blob/master/views/del_cat_loc.slim#L12


No database methods module? Also, your methods mostly return the array of hashes from the database – as opposed to returning an array of objects. I'd like to see demonstration of this at least in 2-3 places before you move on to the project.


Tests?


Documentation is missing entirely (Well, not entirely 😄 – but close!).

@uanwar88
Copy link
Owner

Thanks for the feedback. I've addressed your comments.
I'll keep in mind to assign the variables in the route for next time. The way it's set up currently, if I were to implement it in the route, it would fetch all the categories and locations for every action. This is because all my logic is handled in a separate action.slim file. One compromise would be to assign the variables in action.slim.

The module for Category and Location classes now works.

I added 1 test. Not sure if it's right or useful. I sort of felt like I was just setting it up to pass, which would defeat my purpose.

Improved on the documentation.

Thanks. I'll start working on my project model and should statements.

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

No branches or pull requests

2 participants