DeepPupper / intro-data-capstone-musclehub

0 stars 0 forks source link

Nice consistency, but could use a bit of reorganization #1

Open jmcrey opened 6 years ago

jmcrey commented 6 years ago

https://github.com/bennycsp/intro-data-capstone-musclehub/blob/74f5e1c52b2ff1e4d84e5460f479aacc088e57cb/musclehub_project/musclehub.py#L126-L136

I really like how you kept all your queries consistently organized in these statements; it made it much more readable. Similarly, I really like how you used spacing throughout the entire query to show sub-levels. The spacing gave it a beautiful natural flow that was a pleasure to read.

That being said, we may be able to make it even more readable if we reorganize the key words to line up with one another. This is just so everything that is related is in one straight line and the reader doesn't have to go looking for the key-SQL words. So, in this case, it may be a bit "cleaner" if we move the AND at the end of each phrase to line up with the ON at the beginning of each sub-block. Here's an example:

ON visits.first_name = applications.first_name 
AND visits.last_name = applications.last_name
AND visits.email = fitness_tests.email

With this organization, the reader never second-guesses what is being joined -- the ANDs are right in front of them.

However, this granular a level is honestly a matter of preference; the way it was done prior is perfectly acceptable and, arguably, just as readable. So, truly, great job with this query!