Skip to content

persistence ok and zod on db entity #9

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.

Sign up for GitHub

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

persistence ok and zod on db entity #9

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
39a008d
persistence ok and zod on db entity
GorlemZ Sep 13, 2023
6bed6eb
test fix
GorlemZ Sep 13, 2023
abe51db
closing db connection
GorlemZ Sep 13, 2023
9cebf44
apis added (zodexpress)
GorlemZ Sep 14, 2023
234b86b
ok persistence
GorlemZ Sep 14, 2023
1f3e781
better testing with mock
GorlemZ Sep 14, 2023
0b9b195
general update structure
GorlemZ Sep 14, 2023
b9efafe
error message attempt
GorlemZ Sep 21, 2023
4a2e98e
fix on return error
GorlemZ Sep 21, 2023
cd3caee
logRes async update
GorlemZ Sep 21, 2023
e995b1b
error parsing discussion updates
GorlemZ Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ yarn-debug.log*
yarn-error.log*
lerna-debug.log*
.pnpm-debug.log*
postgres-data*

# Diagnostic reports (https://nodejs.org/api/report.html)
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
Expand Down
9 changes: 4 additions & 5 deletions model/result.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { date, z } from "zod";
import { z } from "zod";

export const results = z.object({
export const result = z.object({
game_date: z.coerce.date(),
result: z.enum(["WIN", "LOSE", "DRAW"]),
});

export type Result = z.infer<typeof results>;

export const resultArray = z.array(results);
export type Result = z.infer<typeof result>;
export const resultArray = z.array(result);
export type ResultArray = z.infer<typeof resultArray>;
29 changes: 20 additions & 9 deletions routes/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { zodiosApp } from "@zodios/express";
import { makeApi } from "@zodios/core";
import { z } from "zod";
import { read } from "../model/move";

import {
welcome,
generateComputerMove,
playLogic,
allGamesParsed,
} from "../service/game";
import { results } from "../model/result";
import { ResultArray, result } from "../model/result";

export const apis = makeApi([
{
Expand All @@ -24,7 +25,7 @@ export const apis = makeApi([
path: "/allgames",
description: "Get all the past games result and timestamps",
response: z.object({
game_history: z.array(results),
game_history: z.array(result),
}),
},
{
Expand Down Expand Up @@ -59,20 +60,30 @@ app
})
.get("/allgames", async (_, res) => {
const allResults = await allGamesParsed();
return res
.status(200)
.json({
game_history: allResults,
})
.end();
if (allResults instanceof Error) {
return res
.status(404)
.json({
game_history: allResults.message,
})
.end();
} else {
return res
.status(200)
.json({
game_history: allResults,
})
.end();
}
})
.post("/play/:move", async (req, res) => {
const userMove = read(req.params.move);
const computerMove = generateComputerMove();
const outcome = await playLogic(userMove, computerMove);
return res
.status(200)
.json({
result: playLogic(userMove, computerMove),
result: outcome,
})
.end();
});
39 changes: 19 additions & 20 deletions service/game.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,39 +1,38 @@
import { match, P } from "ts-pattern";
import { read, Move } from "../model/move";
import { lastGame, allGames } from "../sql/db";
import { Result, resultArray, results } from "../model/result";
import { logRes } from "../sql/db";
import { ResultArray, resultArray, result } from "../model/result";
import { logRes, allGames, lastGame } from "../sql/db";

export function generateComputerMove(): Move {
return read(String(Math.round(Math.random() * 2)));
}
export function playLogic(userMove: Move, computerMove: Move): string {
return match([userMove, computerMove])
export async function playLogic(
userMove: Move,
computerMove: Move
): Promise<string> {
const outcome: string[] = match([userMove, computerMove])
Copy link
Contributor

Choose a reason for hiding this comment

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

suggerirei di fare destructuring tipo

Suggested change
const outcome: string[] = match([userMove, computerMove])
const [result, resultMessage] = match([userMove, computerMove])

per evitare di usare outcome[0] e outcome[1] nel seguito, che mi sembrano meno leggibili

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Che gioia, non avevo proprio considerato che si potesse usare destructuring in TS

.with(
["Rock", "Scissors"],
["Scissors", "Paper"],
["Paper", "Rock"],
() => {
logRes("WIN");
return "You Win!!!";
}
() => ["WIN", "You Win!!!"]
)
.with(
P.when(() => userMove === computerMove),
() => {
logRes("DRAW");
return "It's a Draw!";
}
() => ["DRAW", "It's a Draw!"]
)
.otherwise(() => {
logRes("LOSE");
return "You lose :< ";
});
.otherwise(() => ["LOSE", "You lose :< "]);
const isDbWritten = await logRes(outcome[0]);
if (isDbWritten === null) {
return outcome[1];
} else {
return `${outcome[1]} ... but I couldn't save this result because something went wrong`;
}
}

export async function welcome(): Promise<string[]> {
const res = ["Wanna play? Your move (0: Rock, 1: Paper, 2: Scissors)"];
const lastGameParsed = results.safeParse(await lastGame());
const lastGameParsed = result.safeParse(await lastGame());
if (lastGameParsed.success) {
res.push(
"Our last game timestamp is : " + lastGameParsed.data.game_date + "\n"
Expand All @@ -50,11 +49,11 @@ export async function welcome(): Promise<string[]> {
}
}

export async function allGamesParsed(): Promise<Result[]> {
export async function allGamesParsed(): Promise<Error | ResultArray> {
Copy link
Contributor

Choose a reason for hiding this comment

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

tendenzialmente in TS è più idiomatico fare throw nel caso di errori, generando quindi una Promise fallita: perciò il tipo qui può restare solo Promise<ResultArray>

e poi fai un try/catch dove fai await di questa promise

nulla vieta di restituire invece un'unione, e noi in progetti passati usavamo per esempio fp-ts con TaskEither per usare un Either nel risultato, ma ora di solito facciamo throw in casi del genere (non rende esplicito l'errore nel tipo, naturalmente, quindi aumenta il rischio di scordarsi di gestirlo, ma in pratica gli errori da gestire esplicitametne sono pochi nelle nostre applicazioni tipiche)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ho scelto di restituire Promise<Error | ResultArray> perchè in questo modo mi viene più semplice estrarre il messaggio di errore dal chiamante. Ho provato anche a lavorare sulla Promise fallita, ma il messaggio di errore che mi arriva è quello di zod e non quello a monte. Questo probably è assimilabile all'uso di un Either senza importare fp-ts, immagino. Mi potresti mostrare uno snippet di come lo scriveresti pls?

Copy link
Contributor

Choose a reason for hiding this comment

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

Tendenzialmente farei così:

  1. qui userei .parse invece di .safeParse e quindi cambierei il tipo di ritorno in Promise<ResultArray>
  2. dove lo usi farei 
    try {
  const allResults = await allGamesParsed();
  return res
    .status(200)
    .json({
      game_history: allResults,
    })
    .end();
} catch (error) {
    ...
}

Il problema è che error avrà tipo unknown e non Error, perché in JS si può fare throw anche di cose che non sono Error. Quindi poi per accedere al campo .message devi fare un instanceof.

In pratica però l'errore sarà lo stesso che hai nel campo .error del risultato di safeParse, perché parse si limita a usare safeParse e poi throw. Quindi non dovrebbe essere un messaggio diverso, ma non so se ho capito "il messaggio di errore che mi arriva è quello di zod e non quello a monte".

(In generale mi torna che sia meglio usare safeParse per assicurarsi di gestire l'errore, ma in caso come questo, in cui l'errore avviene solo in caso lo schema del DB è disallineato rispetto al codice, quindi in caso di bug, ci sta fare un parse diretto per semplicità.)

const all = resultArray.safeParse(await allGames());
if (all.success) {
return all.data;
} else {
return [];
return all.error;
}
}
2 changes: 1 addition & 1 deletion sql/db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export async function logRes(res: string) {
return await db.none(addResult, [res]);
}

export async function allGames() {
export async function allGames(): Promise<any[]> {
return await db.any(
"SELECT game_date, result FROM results ORDER BY game_date"
);
Expand Down