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

📊 Improving Analysis Code #40

Open
dpshelio opened this issue Dec 2, 2024 · 3 comments
Open

📊 Improving Analysis Code #40

dpshelio opened this issue Dec 2, 2024 · 3 comments
Labels
week09 Structure and design

Comments

@dpshelio
Copy link
Contributor

dpshelio commented Dec 2, 2024

Open the repository for this exercise. You will find a brief description of each example in its README.md file.

  1. Discuss the code with your group. What problems do you see with it? Why?
  2. Write below your comments mentioning your group number. For now, focus on the
    problems, and don't make any changes yet!
@dpshelio dpshelio added the week09 Structure and design label Dec 2, 2024
@EzAubergine
Copy link

EzAubergine commented Dec 2, 2024

From Group#12
Problems:

  1. Inputting of lines (from reading files) can be written as a function since the codes are repeated. e.g. making a 'read_file_by_lines' function
  2. Comments should be added to at least explain the main purpose and steps of analysis
  3. 'i' and 'j' in loops should be replaced with meaningful iterator (e.g. 'result' in 'results')
  4. Renaming workflows with more meaningful names (state the differences), or explain the usage difference at the start of document

@L-Amos
Copy link

L-Amos commented Dec 2, 2024

  • Lots of reused code across the two files.
  • Comments just say what the code does, not why.
  • In the second file, previous code is commented out for no discernible reason; if you need to return to this previous code you can revert to a previous commit.
  • Unnecessary import.
  • In many cases list comprehension can be used without losing readability.
  • No useful docstrings or comments, so difficult to understand the purpose or context of the code.

Group 9

@cansuoktem
Copy link

cansuoktem commented Dec 2, 2024

From Group 15:

  1. There is repeated code for reading the files, all of this should be written in one function
  2. Not necessary to import all of math
  3. Reused functionality across both workflows, logic could be separately encapsulated, could be inherited
  4. There is not enough comments explaining
  5. Can replace loops with iterator

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

No branches or pull requests

4 participants