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

Refactor 'generate_csv()' #9

Closed
johnmhoran opened this issue Oct 28, 2017 · 3 comments
Closed

Refactor 'generate_csv()' #9

johnmhoran opened this issue Oct 28, 2017 · 3 comments
Assignees

Comments

@johnmhoran
Copy link
Member

I think we might be able to reduce the 38 or so lines used in generate_csv() to construct the .csv tuple down to around 13 lines by using a ternary/conditional expression, e.g.,

new = '' if delta['category'] == 'removed' else delta['new']['path']

Initial testing suggests this works as expected -- all 45 tests pass. The refactoring would be applied here:

for delta in deltas:
category = delta['category']
if delta['category'] == 'added':
new = delta['new']['path']
new_filename = delta['new']['name']
new_sha1 = delta['new']['sha1']
new_size = delta['new']['size']
new_type = delta['new']['type']
new_orig = delta['new']['original_path']
old = ''
old_filename = ''
old_sha1 = ''
old_size = ''
old_type = ''
old_orig = ''
elif delta['category'] == 'removed':
new = ''
new_filename = ''
new_sha1 = ''
new_size = ''
new_type = ''
new_orig = ''
old = delta['old']['path']
old_filename = delta['old']['name']
old_sha1 = delta['old']['sha1']
old_size = delta['old']['size']
old_type = delta['old']['type']
old_orig = delta['old']['original_path']
else:
new = delta['new']['path']
new_filename = delta['new']['name']
new_sha1 = delta['new']['sha1']
new_size = delta['new']['size']
new_type = delta['new']['type']
new_orig = delta['new']['original_path']
old = delta['old']['path']
old_filename = delta['old']['name']
old_sha1 = delta['old']['sha1']
old_size = delta['old']['size']
old_type = delta['old']['type']
old_orig = delta['old']['original_path']

@johnmhoran johnmhoran self-assigned this Oct 28, 2017
@steven-esser
Copy link
Contributor

We should be able to refactor this much more. Your ideas are good, but I am almost certain there is more that we can do.

Right off the bat, the fact that we have to take 12 lines per 'category' to set each variable is very ugly. Also delta['old']['sha1'] is not a great way to reference values either.

Since we are working with the dictionary from to_dict, there is probably an easier way to go from dictionary values to csv.

Give it a try and see what you can do. I can try and write up some examples if you need me too, but you will have to give me some time to dig around and test possible solutions.

@johnmhoran
Copy link
Member Author

Thanks @MaJuRG -- I'll tackle #9 as you suggest and see what I can come up with. 👍

johnmhoran added a commit that referenced this issue Oct 31, 2017
  * Reduces 39-line code block to 12 lines.

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
steven-esser added a commit that referenced this issue Oct 31, 2017
Refactor generate_csv() using ternary operators #9
@steven-esser
Copy link
Contributor

merged, closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants