Full review of repository code by Nazar Pohonchuk - #1
Conversation
Add Blog and blog page.
npogoncuk
left a comment
There was a problem hiding this comment.
Недоліки є. Зазвичай це організація коду (все в одній папці, тай буває багато багато коду в одному файлі). Але загалом проект гарний іпрацює, тому МОЛОДЕЦЬ, продовжуй роботу!
| import logging | ||
| from math import ceil | ||
| from applicationinsights.flask.ext import AppInsights | ||
| from flasgger import Swagger | ||
| from config import Config | ||
| from flask import Flask, render_template, redirect, url_for, flash, request, abort | ||
| from flask_sqlalchemy import SQLAlchemy | ||
| from flask_bcrypt import Bcrypt | ||
| from flask_migrate import Migrate | ||
| import emoji | ||
| from flask_login import ( | ||
| UserMixin, | ||
| login_user, | ||
| LoginManager, | ||
| current_user, | ||
| logout_user, | ||
| login_required, | ||
| ) |
There was a problem hiding this comment.
Ти можеш згрупувати оператори імпорту за типами (наприклад, імпорт, пов'язаний з Flask-related imports, database-related imports, form-related imports), щоб покращити читабельність.
Також зверни уваги чи всі імпорти використовуються у цьому файлі.
There was a problem hiding this comment.
Фактиш, бро, обов'язково зроблю це
| app.config['SECRET_KEY'] = os.environ['SECRET_KEY'] | ||
| app.config['APPINSIGHTS_INSTRUMENTATIONKEY'] = os.environ['APPINSIGHTS_INSTRUMENTATIONKEY'] | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.environ['SQLALCHEMY_DATABASE_URI'] | ||
| app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = True |
There was a problem hiding this comment.
Замість того, щоб встановлювати параметр конфігурації SQLALCHEMY_TRACK_MODIFICATIONS у значення True, можеш спробувати встановити його у app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False, щоб вимкнути відстеження модифікацій. Це може покращити продуктивність.
| @app.route("/", methods=("GET", "POST"), strict_slashes=False) | ||
| @app.route("/home", methods=("GET", "POST"), strict_slashes=False) | ||
| def home(): | ||
| return render_template("home.html",title="Home") | ||
| @app.route("/cats", methods=("GET", "POST"), strict_slashes=False) | ||
| def cats(): | ||
| random_cat_url = get_random_cat() | ||
| random_cat_fact = get_random_cat_fact() | ||
| return render_template("cats.html",title="Cats", random_cat_url=random_cat_url, random_cat_fact=random_cat_fact) | ||
| @app.route("/api_cats_fact") |
There was a problem hiding this comment.
Думаю, варто згрупувати route functions і пов'язані декоратори на основі їхньої мети ( authentication routes, API routes, view routes).
There was a problem hiding this comment.
Теж вірно підмітив, думаю розбити по файлам це
| random_cat_url = get_random_cat() | ||
| return random_cat_url | ||
| @app.route("/login/", methods=("GET", "POST"), strict_slashes=False) |
There was a problem hiding this comment.
Мені здається, шо ти можеш видалити параметр strict_slashes=False з декораторів app.route, оскільки він не є необхідним. Його варто застосовувати тільки якщо ти хочеш обробляти URL-адреси з кінцевими слешами або без них по-різному.
There was a problem hiding this comment.
Розглянь можливість відокремлення route functions в окремі файли або пакети для кращої організації проекту
| from datetime import datetime | ||
| class User(UserMixin, db.Model): | ||
| __tablename__ = "user" |
There was a problem hiding this comment.
За замовчуванням SQLAlchemy автоматично генерує назви таблиць на основі назви класу. Тому вказувати tablename = "user" не обов'язково. Ім'я таблиці буде виведено як "user" з імені класу.
| class Post(db.Model): | ||
| id = db.Column(db.Integer, primary_key=True) | ||
| title = db.Column(LONGTEXT, nullable=False) |
There was a problem hiding this comment.
При визначенні типів стовпців краще використовувати типи SQLAlchemy безпосередньо (db.String, db.Integer і т.д.) замість того, шоб імпортувати специфічні для діалекту типи (LONGTEXT). SQLAlchemy виконає відповідне зіставлення з базовою базою даних.
| id = db.Column(db.Integer, primary_key=True) | ||
| username = db.Column(db.String(80), unique=True, nullable=False) | ||
| email = db.Column(db.String(120), unique=True, nullable=False) | ||
| password = db.Column(db.String(300), nullable=False, unique=True) |
There was a problem hiding this comment.
Стовпчик пароля не повинен мати обмеження unique=True, оскільки воно не є необхідним для унікальності пароля.
| password = db.Column(db.String(300), nullable=False, unique=True) | ||
| date_created = db.Column( | ||
| db.DateTime, nullable=False, default=datetime.utcnow) | ||
| posts = db.relationship('Post', backref='author', lazy=True) |
There was a problem hiding this comment.
Зв'язок між сутностями User і Post визначається за допомогою атрибута posts у класі User. Подумай про додавання каскадної опції до цього зв'язку, якщо пости мають видалятися при видаленні користувача.
There was a problem hiding this comment.
Хм, подумаю, навіть не думав про це, але і юзера видалити не можна поки =)
| }, follow_redirects=True) | ||
| assert response.status_code == 200 | ||
| assert b'Hello, testuser!' in response.data |
There was a problem hiding this comment.
Замість того, шоб перевіряти наявність певних байтів у даних відповіді (b'Hello, testuser!'), подумай про використання більш змістовних асертів. Наприклад, ти можеш перевіряти чи відповідь містить ім'я користувача у вмісті HTML.
There was a problem hiding this comment.
Це теж не погана практика, але з тестами ще потім розберуся
Lab 4