0u714w / ebay_scraper

A Django project that scrapes Ebay for listing data
1 stars 1 forks source link

Structure #2

Open jakeberg opened 4 years ago

jakeberg commented 4 years ago

https://github.com/dougenas/ebay_scraper/blob/a7bbe86703d6a5401042a17189e9039e9aa57165/multiply/views.py#L1-L103

This file seems to be doing a lot more than just passing off views. I would start by breaking out the helper functions you have here into a folder called /services. Each one of the functions should have it's own file and ideally unit tests that go with it. This would give you an opportunity to break up something like create_csv even more.

I can't remember if this is best practice in Python, but in JavaScript I would have each one of these arrays be generated by it's own function or method:

item_name = []

would be

item_name = generate_item_names()

Its a good way to think about breaking up your code.

Last thing is that you might want to give thought to adding urls and things to environment variables. That way there would be no code change if you need to change it once deployed.

jakeberg commented 4 years ago

Someone told me once that you want to be able to read your code like a story, so it might be counter-intuitive to break something up into a bunch of functions, but that way you'd be able to read each function name and know exactly how it is generating the variable. With what you have now, I have to look at the variable then find the for loop that it is being generated from.

0u714w commented 4 years ago

Man, thank you for this. I will look into making some changes.

jakeberg commented 4 years ago

Each one of the functions should have it's own file

A service might have helper functions within that file. you don't need a file for EVERY function. Just wanted to make that clear.

0u714w commented 4 years ago

Created a /services folder and helpers.py for all my helper functions.

I'm having some trouble storing the urls as environment variables. Are urls valid data types for environment variables? I'll keep working on it.