-
Notifications
You must be signed in to change notification settings - Fork 20
C22 Wei and Beenish #10
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
base: main
Are you sure you want to change the base?
Changes from 16 commits
ad158b5
597d34c
cb8eb34
f1433e4
9856a3f
c02f9bf
dc1f1fa
968179e
c9c7631
93abc66
9e3d33e
65111b2
2c4dedc
5310ed1
a8a5c53
b11ec73
0fdc1a2
5dee1a8
e949917
69d4628
49c96a6
fc70d52
410610c
d49aaed
85cd85d
5f2e3af
5d3a50a
c63548a
411075f
d0c56c7
e611944
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| from flask import Flask | ||
| from .routes.planet_routes import planets_bp | ||
|
|
||
|
|
||
| def create_app(test_config=None): | ||
| app = Flask(__name__) | ||
|
|
||
| app.register_blueprint(planets_bp) | ||
| return app |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| from flask import Flask | ||
|
|
||
| class Planet: | ||
|
|
||
| def __init__(self,id,name,description,galaxy): | ||
| self.id = id | ||
| self.name = name | ||
| self.description = description | ||
| self.galaxy = galaxy | ||
|
|
||
| def to_dict(self): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Nice job incorporating this refactor from the live code. |
||
| return { | ||
| "id": self.id, | ||
| "name": self.name, | ||
| "description": self.description, | ||
| "galaxy": self.galaxy | ||
| } | ||
|
Comment on lines
+11
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I prefer to stick with uniform 4-space indented wrapping rather than trying to align to the enclosing character. Generally the closing brace should align with either the first line (pre-indent), or the content lines (1-level indent). As written, it's a bit floaty. return {
"id": self.id,
"name": self.name,
"description": self.description,
"galaxy": self.galaxy
} |
||
|
|
||
|
|
||
| mercury = Planet(1,"Mercury","first planet from the sun","Milkyway") | ||
|
||
| venus = Planet(2,"Venus","second planet from the sun, hottest planet","Milkyway") | ||
| earth = Planet(3,"Earth","HOME PLANET","Milkyway") | ||
| mars = Planet(4,"Mars","has the highest mountain","Milkyway") | ||
| jupiter = Planet(5,"Jupiter","has many moons","Milkyway") | ||
| saturn = Planet(6,"Saturn","has many rings","Milkyway") | ||
| uranus = Planet(7,"Uranus"," the coldest planet in our Solar System","Milkyway") | ||
| neptune = Planet(8,"Neptune","the farthest planet from the Sun","Milkyway") | ||
|
|
||
| planets = [mercury, venus, earth, mars, jupiter, saturn, uranus, neptune] | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| from flask import Blueprint, make_response, abort | ||
| from ..models.planet import planets | ||
|
||
|
|
||
| planets_bp = Blueprint("planets_bp",__name__,url_prefix="/planets") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 Spaces after argument commas. planets_bp = Blueprint("planets_bp", __name__, url_prefix="/planets") |
||
|
|
||
| @planets_bp.get("") | ||
| def get_all_planets(): | ||
| planets_response = [] | ||
| for planet in planets: | ||
| planets_response.append(planet.to_dict()) | ||
|
Comment on lines
+38
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a great opportunity to try to use a list comprehension here. planets_response = [planet.to_dict() for planet in planets] |
||
| return planets_response | ||
|
|
||
| @planets_bp.get("/<planet_id>") | ||
| def get_one_planet(planet_id): | ||
| planet = validate_planet(planet_id) | ||
| return planet.to_dict(),200 | ||
|
||
|
|
||
|
|
||
| def validate_planet(planet_id): | ||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| response = {"message": f"{planet_id} is not valid"} | ||
| abort(make_response(response, 400)) | ||
|
|
||
| for planet in planets: | ||
| if planet_id == planet.id: | ||
| return planet | ||
|
|
||
| response = {"message": f"{planet_id} is not found"} | ||
| abort(make_response(response, 404)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice to see that you discussed your coworking needs and preferences! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: include a space after commas in parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at your data, consider setting a default value for the
galaxyparameter.