London10 | Seyyednavid Hejazijouybari | Full-Stack-Project-Assessment | Level 999 #365
Conversation
seyyednavid
changed the title
seyyednavid
changed the title
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
…outer and also change style
…tead of one generic rating
| throw new Error("Something went wrong!"); | ||
| } | ||
| const data = await response.json(); | ||
| getDeleteMessage(data.message); |
There was a problem hiding this comment.
It's not super clear from the name what getDeleteMessage does - could you think of a more clear name to show what this means?
| } | ||
| }; | ||
|
|
||
| const voteDownHandler = async () => { |
There was a problem hiding this comment.
This looks very similar to the voteUpHandler - can you work out how to share their code in one function, rather than duplicate it?
| const VideoForm = ({ getAllVideos }) => { | ||
| const [title, setTitle] = useState(""); | ||
| const [url, setUrl] = useState(""); | ||
| const [backendMessage, setBackendMessage] = useState(""); |
There was a problem hiding this comment.
What's the backend message? Can you think of a more clear name for this, or add a comment explaining what it's for?
| } else if (response.status === 409) { | ||
| const existingError = await response.json(); | ||
| setErrorMessage(existingError.message); | ||
| } else if (response.status !== 201) { |
There was a problem hiding this comment.
Given you've been doing lots of === checks before, I'd more expect you to handle the === 201 case here, and for the else to be "all other status codes", rather than the other way around. Your core here works, but it's kind of surprising, which makes it slightly harder to read and work with. People are really good at spotting patterns, so breaking that pattern can be confusing.
| const [search, setSearch] = useState(""); | ||
| const [videoSearch, setVideoSearch] = useState([]); |
There was a problem hiding this comment.
It's not super clear from the naming what videoSearch is and how it's different from search - can you think of clearer names to make their differences and roles clear?
Or is there a way you could even get rid of one of the states, and not need them both?
|
|
||
| useEffect(() => { | ||
| if (search === "") { | ||
| return; |
There was a problem hiding this comment.
If you've already searched for something, then get rid of the search term, what will be displayed?
|
|
||
| CREATE TABLE videos ( | ||
| id VARCHAR(255) PRIMARY KEY, | ||
| Date VARCHAR(255), |
There was a problem hiding this comment.
Postgres has specific date types that are more suitable to use than VARCHAR(255), I'd recommend using one as it will allow you to do things like comparisons, or extracting a month, more easily in the future :)
| CREATE DATABASE videos_recommendation; | ||
|
|
||
| CREATE TABLE videos ( | ||
| id VARCHAR(255) PRIMARY KEY, |
There was a problem hiding this comment.
We typically use SERIAL as a type of ids, rather than VARCHARs - can you research SERIAL and work out what it does and why it's useful?
| let filteredVideos = [...allVideos.rows]; | ||
| // Apply search filter if search query is provided | ||
| if (search) { | ||
| filteredVideos = filteredVideos.filter((video) => |
There was a problem hiding this comment.
Can you do this filtering in SQL rather than in javascript? Also the sorting.
Both work, but doing so in SQL will be faster.
| ); | ||
| } | ||
| //Apply order filter | ||
| if (order === "desc") { |
There was a problem hiding this comment.
I think you're sorting on both the client side and the server side - do you need to do both? What are the advantages/disadvantages of doing it in each place?
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
Signed-off-by: Seyyednavid Hejazijouybari <[email protected]>
No description provided.