NickPerlich / Bettermo

1 stars 1 forks source link

Code Review Comments Ivan Nghi #14

Open ivannghi opened 11 months ago

ivannghi commented 11 months ago

You could separate users from purchases and balance endpoints in your code, for organization. You could have the post payment automatically add charge to all users in a group instead of just one user to another. Or you could separate that into two endpoints where you can have that full group split charge, and keep the user to user charge. You can add comments to the code, so it can be better communicated what is being done. Some functions are unclear especially with identically matching function names. You can use different function names, some of the functions are similarly names “post_deliver_bottles” which doesn’t match the description. Return statements on post calls should return ‘OK’ i believe and get calls return values, might be unnecessarily returning values. Organize code to have classes together at the top of file. Remove unused imports Name some of your items selected like: ROUND(:price / total_groupmates.total,2) in total group purchases for example. Format each function more consistently, makes it easier to read and more organized. Be consistent across sql calls with variable names and such, for readability. For the comments that you already have, be descriptive so that someone who has no background knowledge of your service can understand functions. Some comments are confusing, you have #create transaction and #create purchase transaction, but not detailed anywhere what the difference is. Some empty lines could be removed to condense and reduce size of code. And Some empty files in the repository.