Skip to content
Snippets Groups Projects

Implementation of Project Week 4

Open Implementation of Project Week 4
week4 into master
5 unresolved threads
Open yuhou2 requested to merge week4 into master
5 unresolved threads

Created the notes API and connected them to the website. Updated the front end of the project and added a new page about the project and me.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
music-search-library-app/src/components/AlbumsList.js
10 10 import FavoriteIconOutlined from '@material-ui/icons/FavoriteBorderOutlined';
11 11 import Favorited from '@material-ui/icons/Favorite';
12 import CommentIcon from '@material-ui/icons/Comment';
13 import { useState, useEffect, useRef } from "react";
12 14
13 15 const AlbumsList = ({ albums, auth: { user, isAuthenticated }, loadUser }) => {
14 async function onChangeFavorite() {
16 const [albumsList, setalbumsList] = useState(albums)
17 const [itemList, setItemlist] = useState(albums.items)
18
19 function getIndex(id) {
20 return albums.items.findIndex(obj => obj.id === id);
21 }
22
23
24 async function onChangeFavorite(id, image_url) {
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/Login.js
    3 3 import { connect } from 'react-redux';
    4 4 import PropTypes from 'prop-types';
    5 5 import { login } from '../actions/auth';
    6 import { makeStyles } from '@material-ui/core/styles';
    7
    8 const useStyles = makeStyles(theme => ({
    9
    10
    11 }));
    6 12
    7 13 const Login = ({ login, isAuthenticated }) => {
    14 const classes = useStyles();
    • yuefeng3
      yuefeng3 @yuefeng3 ·
      Developer

      It seems like you have unused variable classes and empty useStyle. It would be better to remove them to improve readability and reduce confusion.

    • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/Note.js 0 → 100644
    27
    28 const useStyles = makeStyles((theme) => ({
    29
    30 demo: {
    31 backgroundColor: theme.palette.background.paper,
    32 },
    33 title: {
    34 margin: theme.spacing(4, 0, 2),
    35 },
    36 margin: {
    37 margin: theme.spacing(1),
    38 },
    39 }));
    40
    41
    42 function Note ({ auth: { user }, loadUser }) {
    • yuefeng3
      yuefeng3 @yuefeng3 ·
      Developer

      You have a lot of functions inside this component function, some even has functions inside them. It would be better for you to add some comment explaining their responsibility to improve readability and maintainability.

    • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/abouts.js 0 → 100644
    163 }
    164 />
    165 <CardMedia
    166 align="center"
    167 className={classes.media}
    168 image={elem.imageUrl}
    169 />
    170 <CardActions>
    171 <Grid container alignItems="center"
    172 justify="space-around"
    173 marginTop="22"
    174 flex-direction="row"
    175 spacing={2}
    176 >
    177
    178 <Button className={classes.textcolor} size="medium" onClick={() => { window.open(elem.linkedinLink, '_blank') }} >
  • yuefeng3
    yuefeng3 @yuefeng3 started a thread on the diff
    • music-search-library-app/src/components/abouts.js 0 → 100644
    195 <ExpandMoreIcon />
    196 </IconButton>
    197
    198 </Grid>
    199 </CardActions>
    200 <Collapse in={expandedIdx === idx} timeout="auto" unmountOnExit>
    201 <CardContent>
    202 <Typography paragraph>{`${elem.description}`}</Typography>
    203 </CardContent>
    204 </Collapse>
    205 </Card>
    206 </Grid>
    207 );
    208
    209
    210 return (
    • yuefeng3
      yuefeng3 @yuefeng3 ·
      Developer

      You have a really huge component here. I would suggest you to use some code formatter to organize it, and you might want to consider separate them into smaller components to improve maintainability.

    • Please register or sign in to reply
  • yuefeng3
    yuefeng3 @yuefeng3 ·
    Developer

    Peer Review Report

    1-This question references the Code Review Checklist available at course website. Select one item from Item 4 (Non Functional requirements) in the checklist, and answer the following questions:

    • Maintainability
    • There are rooms to improve
    • Some unused code and series of functions without comments could create confusion. It might require extra effort in the future to extend or maintain the code.

    2-Please select one item from the Item 5 (OOAD Principle), and answer the following questions:

    • Interface segregation
    • Should put in some effort for this principle
    • The about component is really lengthy and the format is a bit difficult to follow. It might be better to separate it into smaller components.

    3-If applicable, does the author address all the comments you made last week?

    Unused and commented from previous week was removed, but could still organize the code better with some IDE formatter plugins.

    • Please register or sign in to reply