Skip to content

Cedar - Jacy & Kit#15

Open
jacyyang04 wants to merge 38 commits into
Ada-C16:mainfrom
jacyyang04:main
Open

Cedar - Jacy & Kit#15
jacyyang04 wants to merge 38 commits into
Ada-C16:mainfrom
jacyyang04:main

Conversation

@jacyyang04

Copy link
Copy Markdown

No description provided.

@beccaelenzil beccaelenzil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎉 Nice work!

Comment thread app/routes.py Outdated
planets_response = []

for planet in PLANETS:
planets_response.append(vars(planet))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nice use of vars!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I just learned that vars will cause problems when we have a class that connects to our database. We can achieve the same effect by writing a to_json(self) instance method on the Planet class.

Comment thread app/routes.py Outdated
Comment on lines +3 to +22
from flask import Blueprint, jsonify


class Planet():
def __init__(self, id, name, description, xenomorphs=False):
self.id = id
self.name = name
self.description = description
self.xenomorphs = xenomorphs


PLANETS = [

Planet(426, "Nostromo's End", "Hostile weather. Toxic atmosphere. Evidence of civilization.", True),
Planet(224, "JollyPlanet", "Okay. Decent. Will live for long time.", True)

]

planets_bp = Blueprint("planets_bp", __name__, url_prefix="/planets")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor note, consider moving extra spaces:

Suggested change
from flask import Blueprint, jsonify
class Planet():
def __init__(self, id, name, description, xenomorphs=False):
self.id = id
self.name = name
self.description = description
self.xenomorphs = xenomorphs
PLANETS = [
Planet(426, "Nostromo's End", "Hostile weather. Toxic atmosphere. Evidence of civilization.", True),
Planet(224, "JollyPlanet", "Okay. Decent. Will live for long time.", True)
]
planets_bp = Blueprint("planets_bp", __name__, url_prefix="/planets")
from flask import Blueprint, jsonify
class Planet():
def __init__(self, id, name, description, xenomorphs=False):
self.id = id
self.name = name
self.description = description
self.xenomorphs = xenomorphs
PLANETS = [
Planet(426, "Nostromo's End", "Hostile weather. Toxic atmosphere. Evidence of civilization.", True),
Planet(224, "JollyPlanet", "Okay. Decent. Will live for long time.", True)
]
planets_bp = Blueprint("planets_bp", __name__, url_prefix="/planets")

@kaidamasaki kaidamasaki left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job!

It's too bad your planets seem to have a case of xenomorphs though!

Comment thread app/models/planet.py
"id": self.id,
"name": self.name,
"description": self.description,
"xenomorphs": self.xenomorphs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh no!

Comment thread app/routes.py
Comment on lines +48 to +53
##---partial functionality; would like to discuss---##
if name_query:
planets = Planet.query.filter_by(name=name_query)
elif xenomorphs_query:
planets = Planet.query.filter_by(xenomorphs=xenomorphs_query)
##---END---##

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sign up for free to 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.

4 participants