Conversation
Vuictorovna
left a comment
Vuictorovna
left a comment
There was a problem hiding this comment.
Great job, T! Your code provides a straightforward and functional approach to monitoring stock prices and sending alerts. The comments are also quite helpful for understanding the flow of the application.
There are just a few points that could be improved or clarified to make the code even better. These are mainly around enhancing security by storing sensitive information in environment variables, improving readability by removing commented-out code, and increasing maintainability through function decomposition.
Again, excellent work!
| # import os | ||
| import requests | ||
| import time | ||
| # from dotenv import load_dotenv |
There was a problem hiding this comment.
Noticed some commented-out code here. Would it make sense to remove it to keep the codebase clean? 😊
| # average_drop_key = os.environ['AVERAGE_DROP_KEY'] | ||
|
|
||
| # Replace with your IFTTT Webhooks keys and event names | ||
| price_drop_key = 'hk1XqN9c6Lf3fny61cgEOLQb3L_J71xc5h2WpNu9MmP' |
There was a problem hiding this comment.
It might be a good idea to move the IFTTT Webhooks keys to a .env file for added security.
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
I see it's an empty file. Perhaps it might be best to remove it unless it serves a specific purpose?
|
|
||
| Good Luck! | ||
| ## Overview of Existing Code | ||
| This code returns real-time prices for the stocks provided (AAPL, GOOGL, TSLA, MSFT and NKE). With an extended deadline, this application will alert the user(s) to price drops of at least £0.25 GBP, and whether the latest price is lower than the 7-day average for that particular stock. |
There was a problem hiding this comment.
Great move on updating the README to include an app description rather than tasks!
| else: | ||
| print(f"Failed to send alert to {event_name}. Status code: {response.status_code}") | ||
|
|
||
| def fetch_stock_data(symbol): |
There was a problem hiding this comment.
This function has multiple responsibilities. Would it make sense to split it into smaller functions for better readability and maintainability?
| def send_webhook_alert(event_key, event_name, message): | ||
| # Send a webhook alert | ||
| url = f'https://maker.ifttt.com/trigger/{event_name}/with/key/hk1XqN9c6Lf3fny61cgEOMkLhEGT8CNupRK-3l5KgWy' | ||
| playload = {'value1': message} |
There was a problem hiding this comment.
It seems like "playload" might be a typo. Did you mean payload?
2 participants
No description provided.