Skip to content
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

Koodikatselmointi #1

Open
HerberHoover opened this issue May 1, 2023 · 0 comments
Open

Koodikatselmointi #1

HerberHoover opened this issue May 1, 2023 · 0 comments

Comments

@HerberHoover
Copy link

1.5.2023 klo 16.00

Ilman mitään pidempiä aloituksia.

Initialize_databse.py / database_commands.py

koodilla on selkeä tarkoitus, helposti ymmärrettävää sekä funktiot ovat nimetty hyvin.

  • SQL-injektio vaara, käytä paikkamerkkejä (?) kyselyssä ja anna arvot tuple-muodossa esimerkiksi. tästä lisää ja selkeemmin selitettynä sivulla https://hy-tsoha.github.io/materiaali/osa-4/#tietoturva kohta sql-injektio

  • get_course_id funktiossa lisäksi kirjoitusvirhe return "fetcone"

.env hakemisto

en ole varma tämän olemassaolosta, samat tiedot löytyvät muualta koodista.

config.py

Dotenv-kirjaston käyttäminen ympäristömuuttujien lataamiseen on hyvä käytäntö ja joka on myös syy miksi print(".env file not found") ei koskaan tule koska load_dotenv ei aiheuta 'FileNotFoundError' poikkeusta. Tätä lohkoa ei siis ajeta vaikka ei olisi .env tiedostoa. Jos haluat tarkistaa asian voisit käyttää " if not load_dotenv(), koska funktion palauttama ilmoitus tiedoston puutteesta on false.

DATABASE_NAME = getenv("DATABASE") or "db.sqlite3" Tämä on hyvä käytäntö joka tekee koodista joustavamman. Jos .env-tiedostoa ei löydy tai DATABASE-ympäristömuuttujaa ei ole asetettu, koodi käyttää silti oletusarvoa db.sqlite3 tässä tapauksessa.

Käyttöliittymä ( UI )

Käyttöliittymän koodi on jaettu hyvin eri luokkiin ja metodeihin. Huomasin myös että sinulla oli ongelmia sen kanssa

Ohjelman käyttöliittymässä olevat ongelmat ovat helposti korjattavissa jotta komento poetry run invoke start toimii. (jos poistaa koodin kommenteista index.py tiedostossa)

tällä hetkellä komento taisi ilmoittaa että 'No module named 'create_login_view'

tällä hetkellä tuonti lausekkeet yrittävät tuoda koko tiedostoa eikä itse luokkaa
import create_login_view as login_view
import create_guest_view as create_guest_view

tuontikäskyt pitää muotoilla tuomaan luokat joka tapahtuu seuraavasti (relative import)
from .create_login_view import LoginView as login_view (Jos haluaa käyttää muuttujaa)
from .create_guest_view import FindGroupView as create_guest_view.

tämä herätti seuraavan ongelma ohjelman kanssa start funktiossa jonka lisäksi 'shadow_login' sekä show_guest funktioissa sama ongelma. seuraava pätkä start funktiosta
self._current_view = login_view.LoginView(self._root, handle... )

kohta 'login_view.LoginView' käyttää .LoginViewtä funktioina koska ohjelma yrittää kutsua luokkaa kaksi kertaa putkeen. Voisi ajatella että siinä lukisi LoginView.LoginView(self.root,...) tällä hetkellä. Jos haluaa käyttää login_view muuttujaa kohta .LoginView tulee ottaa pois, mutta mielestäni muuttujaa ei olisi tarpeellista käyttää, eli self._current_view = LoginView(...) olisi mitä ehdottaisin

Huomasin myös ongelmasi # importing modules is hard.
Suosittelen käyttämään 'suhteellista tuontia?' / relative import. Näin ei tarvitse käyttää new_path = os_path.dirname((file)) + "/../" sys_path.append(new_path) joka kerta.
Tähän sinun tulisi tehdä uusi hakemisto src juureen jonne sijoittaa juuressa sijaitsevat ylimääräiset tiedostot ja lisäksi tehdä tyhjä init.py tiedosto hakemistoon. init.py-tiedosto toimii Pythonille osoituksena siitä, että sen sisältävää hakemistoa tulee käsitellä pakettina. Näin voisit käyttää suhteellista tuontia moduulien tuomiseen. " from hakemisto.database_connection import get_connection ". Tämän olet jo aloittanut database_tools hakemistolla sekä oletan että myös repositories hakemistolla.

toivottavasti en tule sekoittamaan päätäsi näillä, sekä toivottavasti tietoni ei ole väärässä jotta en ehdottele tekemään vääriä asioita.
Lisäksi nimeämisessä voisi olla parempi yhtenäisyys esim. create_guest_view.py tiedoston luokka nimellä
FindGroupView sekoittaa meitä hajamielisiä.

Kaiken kaikkiaan paljon positiivista ja hyvää myös mutta säästelin sanojani niiden kanssa. Oli ilo nähdä ja lukea läpi mitä olit tuottanut. (voit vapaasti kommentoida palautteeseen jos tulee jotain)

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

No branches or pull requests

1 participant