Joel6677 / fullstack-ht

0 stars 0 forks source link

Full Stack projektin katselmointi #1

Open Jakousa opened 3 years ago

Jakousa commented 3 years ago

Full Stack projektin katselmointi

Tässä lyhyt katselmointi Full Stack kurssin projektistasi. Katselmoinnissa esitetyt kommentit ovat parannusehdotuksia, joita voit halutessasi ottaa huomioon tässä tai tulevissa projekteissasi. Mitään muutoksia tähän projektiin ei siis ole pakko tehdä suoritusmerkintää varten.

Käytettävyys

Mitä tein?

  1. Sekoilin eka kaikissa väärissä näkymissä, ohjelma taisi jopa kaatua kerran.
  2. Rekisteröidyin
  3. Kirjauduin
  4. Arvioin yhden viskin

Kokemus

Kunhan löysin itseni pois vääristä näkymistä ja ymmärsin kirjautua ensin sisälle sovellus toimi hienosti!

Huomasin että esim yhden viskin tiedot avatessa välähtää monta "undefined" tekstiä lukemaan. Ne voisi varmastikkin piilottaa

Koodi

Koodi on jäsennelty selkeästi, löydän juuri sieltä komponentit mistä etsinkin. Stylesheettien käyttö näyttää hyvältä ja olet pilkkonut sovelluksen järkevän kokoisiin komponentteihin, hienoa!

Tässä voisit käyttää portsaripatternia (https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html): https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/ChoosePic.jsx#L65

if (result.cancelled) return

Mikäs tiedosto tämä on https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/Collections.jsx tämmöisiä tyhjiä oli muutamakin kappale. Entä tämä https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/Home.jsx

Sisennyksissä on jotain kummallisuuksia, mm. https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/ChoosePic.jsx#L110 ja https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MessageItem.jsx#L84 ja https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyMessages.jsx#L89 ja https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyCollection.jsx#L119

Päätä käytätkö välilyöntejä vai tabeja sisennyksessä ja pysy siinä. Prettier ja eslint työkaluina voivat auttaa sinua pysymään jossain käytännössä.

Git on parempi versionhallinnan työkalu kuin kommentit https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyReviews.jsx#L44-L54

En ole ihan varma mitä täällä tapahtuu, mutta eikö tähän sopisi paremmin vaikkapa userIDs.find() (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyMessages.jsx#L73-L77

Promiseiden käytössä on hieman petrattavaa, await ja then on käytössä ristiin. Yhdessä niiden käyttö aiheuttaa vain sekavaa koodia. Molemmat siis toimivat ja ovat hyviä työkaluja. Mutta ne tekevät saman asian joten ei käy järkeen käyttää molempia samassa paikassa. Käytä vain jompaa kumpaa, älä molempia. Ja näistä kahdesta suosittelen uudempana työkaluna awaitia.

Tässä kaksi vaihtoehtoista esimerkkiä, valitse jompi kumpi ja pitäydy siinä:

const [valueInState, setValueInState] = useState(false)

const jokuPromisenPalauttavaKutenVaikkapaFirebase = () => { return new Promise((res) => res('haluttu')) }

const samaKuinToinen = () => {
  jokuPromisenPalauttavaKutenVaikkapaFirebase().then((palautusArvo) => {
    if (palautusArvo !== 'haluttu') return

    setValueInState(true)
  })
}

// async / await tuotiin javascriptiin vuonna 2017
const samaKuinEnsimmäinen = async () => {
  const palautusArvo = await jokuPromisenPalauttavaKutenVaikkapaFirebase()
  if (palautusArvo !== 'haluttu') return

  setValueInState(true)
}

Kokonaisuus

Äppi näyttää hienolta ja vaikka koodi vaatii vielä refaktorointia ja siistimistä, sen työn jälkeen sovellus olisi varmasti erinomaisessa jamassa!

Toivottavasti jaksat vielä jatkaa sovelluksen parantelua! Siinä on lupausta vaikkapa sovellusportfoliota varten.

Joel6677 commented 3 years ago

Kiitos hyvästä palautteesta, aion tehdä parannukset tässä tuota pikaa.

Kysyisin vielä sitä, että missä vaiheessa harjoitustyön opintopisteet ilmaantuvat?

Ystävällisin terveisin, Joel Heusala

ke 9. kesäk. 2021 klo 15.10 Jami Kousa @.***) kirjoitti:

Full Stack projektin katselmointi

Tässä lyhyt katselmointi Full Stack kurssin projektistasi. Katselmoinnissa esitetyt kommentit ovat parannusehdotuksia, joita voit halutessasi ottaa huomioon tässä tai tulevissa projekteissasi. Mitään muutoksia tähän projektiin ei siis ole pakko tehdä suoritusmerkintää varten. Käytettävyys Mitä tein?

  1. Sekoilin eka kaikissa väärissä näkymissä, ohjelma taisi jopa kaatua kerran.
  2. Rekisteröidyin
  3. Kirjauduin
  4. Arvioin yhden viskin

Kokemus

Kunhan löysin itseni pois vääristä näkymistä ja ymmärsin kirjautua ensin sisälle sovellus toimi hienosti!

Huomasin että esim yhden viskin tiedot avatessa välähtää monta "undefined" tekstiä lukemaan. Ne voisi varmastikkin piilottaa Koodi

Koodi on jäsennelty selkeästi, löydän juuri sieltä komponentit mistä etsinkin. Stylesheettien käyttö näyttää hyvältä ja olet pilkkonut sovelluksen järkevän kokoisiin komponentteihin, hienoa!

Tässä voisit käyttää portsaripatternia ( https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html ):

https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/ChoosePic.jsx#L65

if (result.cancelled) return

Mikäs tiedosto tämä on https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/Collections.jsx tämmöisiä tyhjiä oli muutamakin kappale. Entä tämä https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/Home.jsx

Sisennyksissä on jotain kummallisuuksia, mm.

https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/ChoosePic.jsx#L110 ja

https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MessageItem.jsx#L84 ja

https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyMessages.jsx#L89 ja

https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyCollection.jsx#L119

Päätä käytätkö välilyöntejä vai tabeja sisennyksessä ja pysy siinä. Prettier ja eslint työkaluina voivat auttaa sinua pysymään jossain käytännössä.

Git on parempi versionhallinnan työkalu kuin kommentit https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyReviews.jsx#L44-L54

En ole ihan varma mitä täällä tapahtuu, mutta eikö tähän sopisi paremmin vaikkapa userIDs.find() ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find)

https://github.com/Joel6677/fullstack-ht/blob/f3e1ed626460df8c9fdc86b5ea07ed70a5873039/fullstack-ht-app/src/components/MyMessages.jsx#L73-L77

Promiseiden käytössä on hieman petrattavaa, await ja then on käytössä ristiin. Yhdessä niiden käyttö aiheuttaa vain sekavaa koodia. Molemmat siis toimivat ja ovat hyviä työkaluja. Mutta ne tekevät saman asian joten ei käy järkeen käyttää molempia samassa paikassa. Käytä vain jompaa kumpaa, älä molempia. Ja näistä kahdesta suosittelen uudempana työkaluna awaitia.

Tässä kaksi vaihtoehtoista esimerkkiä, valitse jompi kumpi ja pitäydy siinä:

const [valueInState, setValueInState] = useState(false)

const jokuPromisenPalauttavaKutenVaikkapaFirebase = () => { return new Promise((res) => res('haluttu')) }

const samaKuinToinen = () => {

jokuPromisenPalauttavaKutenVaikkapaFirebase().then((palautusArvo) => {

if (palautusArvo !== 'haluttu') return

setValueInState(true)

}) }

// async / await tuotiin javascriptiin vuonna 2017 const samaKuinEnsimmäinen = async () => {

const palautusArvo = await jokuPromisenPalauttavaKutenVaikkapaFirebase()

if (palautusArvo !== 'haluttu') return

setValueInState(true) }

Kokonaisuus

Äppi näyttää hienolta ja vaikka koodi vaatii vielä refaktorointia ja siistimistä, sen työn jälkeen sovellus olisi varmasti erinomaisessa jamassa!

Toivottavasti jaksat vielä jatkaa sovelluksen parantelua! Siinä on lupausta vaikkapa sovellusportfoliota varten.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Joel6677/fullstack-ht/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEA2N6LME4WSDSZQ5Y6PRALTR5KZRANCNFSM46L5A7GA .