Pine Ainur and Tirhas #17
Conversation
app/routes.py
Outdated
|
|
||
| @solarsystem_bp.route("", methods=["GET"]) | ||
| def handle_solarsystem(): | ||
| solarsystem_response =[vars(planet) for planet in planets] |
app/routes.py
Outdated
| else: | ||
| return "please enter a valid planet", 404 |
There was a problem hiding this comment.
This is a great idea, but it doesn't quite work as intended. If the first planet name does not match, this endpoint will return the 404 message, rather than continuing to look for a matching planet. Moving the 404 return outside of the for loop will result in returning the 404 only if the planet is not found by the for loop.
| else: | |
| return "please enter a valid planet", 404 | |
| return "please enter a valid planet", 404 |
app/routes.py
Outdated
| for planet in planets: | ||
| max_distance = max(distances) |
There was a problem hiding this comment.
The max_distance is the same for every iteration of the for loop - I recommend moving it outside of the loop to improve the efficiency of the algorithm.
| for planet in planets: | |
| max_distance = max(distances) | |
| max_distance = max(distances) | |
| for planet in planets: |
app/routes.py
Outdated
| print(name) | ||
| for planet in planets: | ||
| if planet.name == name: | ||
| return vars(planet) |
There was a problem hiding this comment.
I recommend adding jsonify to the return even in the case where the Flask default behavior is doing the same thing. Using jsonify adds a layer of error protection and also makes it clear that json is the intended behavior.
| return vars(planet) | |
| return jsonify(vars(planet)) |
app/__init__.py
Outdated
|
|
||
| def create_app(test_config=None): | ||
| app = Flask(__name__) | ||
|
|
||
| app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = 'postgresql+psycopg2://postgres:postgres@localhost:5432/solarsystem_db' |
There was a problem hiding this comment.
This is fine here, but in the TaskList project you should make the connection string come from an env variable
| distance_from_sun = db.Column(db.Integer) | ||
| radius = db.Column(db.Integer) | ||
|
|
||
| def to_dict(self): |
| for planet in planets: | ||
| solarsystem_response.append(planet.to_dict()) | ||
|
|
||
| return jsonify(solarsystem_response) |
There was a problem hiding this comment.
For readability I suggest using response codes, even for 200s
| return jsonify(solarsystem_response) | |
| return jsonify(solarsystem_response) , 200 |
| if 'name' not in request_data or 'surface_area' not in request_data or 'orbital_period' not in request_data \ | ||
| or 'distance_from_sun' not in request_data or 'radius' not in request_data: | ||
| return jsonify({'message': "missing data"}),400 | ||
| if type(request_data['name']) != str or type(request_data['surface_area']) != int or\ | ||
| type(request_data['orbital_period']) != int or type(request_data['distance_from_sun']) != int\ | ||
| or type(request_data['radius']) != int : | ||
| return jsonify({'message': "Unsupported data type"}),400 |
There was a problem hiding this comment.
This might make a good helper function.
No description provided.