Skip to content

commit atm - #140

Open
kerenkay wants to merge 5 commits into
osherdp:masterfrom
kerenkay:feature/ATM
Open

commit atm#140
kerenkay wants to merge 5 commits into
osherdp:masterfrom
kerenkay:feature/ATM

Conversation

@kerenkay

Copy link
Copy Markdown
Collaborator

No description provided.

@kerenkaykerenkay closed this Apr 5, 2020
@kerenkaykerenkay reopened this Apr 5, 2020
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
Comment threadATM.py Outdated
@rfire01

Copy link
Copy Markdown
Collaborator

make sure flake 8 pass

Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
with open(filename, 'r') as file:
for line in file:
details = line.strip().split(' ')
atm[details[ID_INDEX]] = details[INDEX_START_OF_PASSWORD_AND_BALANCE:]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving the data as list isn't a good practice.
What happen if you change the way the data look? you need to change everywhere you use it the indexes of each part.
Instead the only place that should be effect is the read file function

Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Comment threadATM.py
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@kerenkay@rfire01@jhb1996@natsala13@IgalKolihman