3
\$\begingroup\$

I've got this custom hook that is suffering from a bad case of violation of SRP This is a simple 5x5 board where a car can move around. Together with a server side check if the move is valid or not.

I'm thinking of moving directions to a useReducer hook. But problem is that I'm calling that using its index when I do things like rotate and move. I'm also putting the x, y and degrees in the same data object.

What's a good way to split this up following React Hooks best practices?

import { useState } from 'react'
import { createContainer } from "unstated-next"
import axios from 'axios'

const useController = () => {
  const directions = [
    {x: 0, y: -1, degrees: 0}, // up
    {x: 1, y: 0, degrees: 90}, // right
    {x: 0, y: 1, degrees: 180}, // down
    {x: -1, y: 0, degrees: 270}, // left
  ]
  const [initiatedGame, setInitiatedGame] = useState(false)
  const [degrees, setDegrees] = useState(0)
  const [direction, setDirection] = useState(0)
  const [position, setPosition] = useState({'x': 0, 'y': 0})
  const [carPlaced, setCarPlaced] = useState(false)

  const chooseDirection = direction => {
    setDirection(direction)
    setDegrees(directions[direction].degrees)
    setInitiatedGame(true)
  }

  const changeDirection = direction => {
    if(direction === -1) {
        setDirection(3)
    } else if(direction === 4) {
        setDirection(0)
    } else {
        setDirection(direction)
    }
  }

  const rotateLeft = () => {
    setDegrees(degrees - 90)
    changeDirection(direction - 1)
  }

  const rotateRight = () => {
    setDegrees(degrees + 90)
    changeDirection(direction + 1)
  }

  const move = () => {
    const root = process.env.REACT_APP_API_ROOT
    const {x, y} = directions[direction]
    axios.post(`${root}/move`, {
        x: position.x + x,
        y: position.y + y
      })
      .then((response) => {
        if(response.data.valid) {
          setPosition({x: position.x + x, y: position.y + y})
        }
      })
  }

  const placeCar = (x, y) => {
    if(!carPlaced) {
      setCarPlaced(true) 
      setPosition({x, y})
    }
  }

  const onKeyDown = e => {
    switch (e.which) {
      case 37: // Left
        rotateLeft()
        break
      case 38: // Up
        move()
        break
      case 39: // Right
        rotateRight()
        break
      default:
        break
    }
  } 

  return {
    rotateLeft,
    rotateRight,
    move,
    degrees,
    position,
    placeCar,
    carPlaced,
    chooseDirection,
    initiatedGame,
    onKeyDown
  }
}

export const Controller = createContainer(useController)
export default useController
\$\endgroup\$
2
  • \$\begingroup\$ "bad case of violation of SRI." what does "SRI" mean? \$\endgroup\$ – Peter Sep 18 '19 at 15:05
  • \$\begingroup\$ @Peter Sorry, meant to say SRP (single responsibility principle) \$\endgroup\$ – dan-klasson Sep 18 '19 at 15:10
4
\$\begingroup\$

I'm trying to address the same issue. What I'm currently doing is splitting my big hooks into smaller hooks, each with a single responsability.

For example, in your example, you could split your onKeyDown, rotateRight and rotateLeft functions into a hook which takes a state with values and setters as only parameter:

const useCarRotation = state => {
  const changeDirection = direction => {
    if(direction === -1) {
        state.setDirection(3)
    } else if(direction === 4) {
        state.setDirection(0)
    } else {
        state.setDirection(direction)
    }
  }

  const rotateLeft = () => {
    state.setDegrees(state.degrees - 90)
    changeDirection(state.direction - 1)
  }

  const rotateRight = () => {
    state.setDegrees(state.degrees + 90)
    changeDirection(state.direction + 1)
  }

  const onKeyDown = e => {
    switch (e.which) {
      case 37: // Left
        rotateLeft()
        break
      case 38: // Up
        move()
        break
      case 39: // Right
        rotateRight()
        break
      default:
        break
    }
  } 

  return {onKeyDown, rotateLeft, rotateRight};
}

You can then use it in your main hook like this:

const useController = () => {
  const directions = [
    {x: 0, y: -1, degrees: 0}, // up
    {x: 1, y: 0, degrees: 90}, // right
    {x: 0, y: 1, degrees: 180}, // down
    {x: -1, y: 0, degrees: 270}, // left
  ]
  const [initiatedGame, setInitiatedGame] = useState(false)
  const [degrees, setDegrees] = useState(0)
  const [direction, setDirection] = useState(0)
  const [position, setPosition] = useState({'x': 0, 'y': 0})
  const [carPlaced, setCarPlaced] = useState(false)

  const chooseDirection = direction => {
    setDirection(direction)
    setDegrees(directions[direction].degrees)
    setInitiatedGame(true)
  }

  const move = () => {
    const root = process.env.REACT_APP_API_ROOT
    const {x, y} = directions[direction]
    axios.post(`${root}/move`, {
        x: position.x + x,
        y: position.y + y
      })
      .then((response) => {
        if(response.data.valid) {
          setPosition({x: position.x + x, y: position.y + y})
        }
      })
  }

  const placeCar = (x, y) => {
    if(!carPlaced) {
      setCarPlaced(true) 
      setPosition({x, y})
    }
  }

  const {onKeyDown, rotateLeft, rotateRight } = useCarRotation({
    degrees,
    direction,
    setDegrees,
    setDirection,
  })

  return {
    rotateLeft,
    rotateRight,
    move,
    degrees,
    position,
    placeCar,
    carPlaced,
    chooseDirection,
    initiatedGame,
    onKeyDown
  }
}

To take your example further, I believe the chooseDirection and placeCar function could be placed in a hook responsible for initializing your values.

I currently keep all useState and useSelector calls into my main hook but I guess they could be put into a separate hook too if you have a lot of them.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.