Please review - #2
Conversation
Code review
… command line further work needed
Nancy-Singleton
left a comment
Nancy-Singleton
left a comment
There was a problem hiding this comment.
Looks good!
On the point about getting user input from the console - if I have misunderstood and that's not what you were looking for, I'd suggest it as a goal anyway! :)
| build-backend = "poetry.core.masonry.api" | ||
|
|
||
| [tool.poetry.scripts] | ||
| test = "main:Test" |
There was a problem hiding this comment.
Nice! This was new to me
| @@ -0,0 +1,59 @@ | |||
|
|
|||
| #! I am aware that I havent quite got there with making the scripts run correctly. WIP. | |||
| #! I am also aware that I have used a mixture of camel/snake/etc will tidy up at the end. | |||
| #! I am aware that I havent quite got there with making the scripts run correctly. WIP. | ||
| #! I am also aware that I have used a mixture of camel/snake/etc will tidy up at the end. | ||
| #TODO change to classes "accounts" and "transactions" | ||
| #TODO is there a way of passing parameters to entry points on console |
There was a problem hiding this comment.
Not sure if I'm reading this right - are you looking to collect user input from the console? You can do that with a built in function in Python :) https://www.w3schools.com/python/python_user_input.asp
…all for the file you want
Nancy-Singleton
left a comment
Nancy-Singleton
left a comment
There was a problem hiding this comment.
Nice work with the classes :)
Next thing to try: can you run the app and accept continuous user input? I.e.:
- You run the app with a task called something like
run - When the app starts, it reads all the transactions from the files into memory, then asks the user for their input
- If they say e.g. List All, then it prints the result
- Then asks the user for their input again
…ilot so that i can get used to using ai
|
|
||
|
|
||
| class Transaction: | ||
| def __init__(self, data): |
There was a problem hiding this comment.
I'd recommend passing in separate variables (e.g. from, narrative) rather than data - then at the point you create a new transaction (read_transactions) you can pass the right bits of data in :)
| for idx, row in enumerate(data, start=1): | ||
| try: | ||
| if filepath.endswith(".json"): | ||
| with open(filepath, "r") as f: |
There was a problem hiding this comment.
I think pulling out some functions called read_json_file, read_csv_file etc would be good for readability here :)
| logger.info("Accounts and balances displayed successfully") | ||
|
|
||
| def list_transactions(account_name, transactions): | ||
| found = False |
There was a problem hiding this comment.
Instead of having this found variable, you can just return early if there's no transactions, with something like this:
transactions_for_user = [tx for tx in transactions if tx.from_account == account_name or tx.to_account == account_name]
transactions_for_user.sort(key=lambda tx: datetime.strptime(tx.date, "%d/%m/%Y"))
if transactions_for_user.len() == 0:
print(f"No transactions found for account: {account_name}")
return
for tx in transactions_for_user:
print(f"{tx.date} | From: {tx.from_account} | To: {tx.to_account} | {tx.narrative} | £{tx.amount:.2f}")
| self.amount = float(amount) | ||
|
|
||
| @staticmethod | ||
| def normalize_date(date_str): |
|
How was the Copilot experience? |
2 participants
No description provided.