関心の分離を意識したコードとは？

Last updated at 2021-11-12Posted at 2021-11-12

これはなに？

筆者がコードレビューを受けた際に学んだ「関心の分離」について記載しています。
もし違っている箇所がありましたら、優しさレベルマックスでコメントしていただけると助かります(ﾏｻｶﾘｺﾜｲ)。

作ったもの

実際にレビューで指摘を受けたコードをもとに今回は説明していきます

内容としては、Stepsの作成です。
StepsはMUI(旧Material-UI)でいうStepperと同じ立ち位置で、弊社で利用しているChakra UIではそれに当たるものがなかった&他のパッケージを適用するにもデザインの制約が厳しかったので今回作成に至りました。

デモ

コード置き場

開発環境

FE: Next.js
UI: Chakra UI

before

import Steps, { Step } from "@/presentationals/register/Steps"
import { Box, Button, Container, Text } from "@chakra-ui/react"
import { NextPage } from "next"
import { useRouter } from "next/dist/client/router"
import React, { useState } from "react"

const Register: NextPage = () => {
  const router = useRouter()
  const steps: Step[] = [{ label: "Step 1" }, { label: "Step 2" }]
  const [activeStep, setActiveStep] = useState<number>(0)
  const handleNext = () => {
    setActiveStep(activeStep + 1)
    router.push("/register?step2=true", undefined, { shallow: true })
  }
  const handleBack = () => {
    setActiveStep(activeStep - 1)
    router.push("/register", undefined, { shallow: true })
  }

  return (
    <Container maxW="container.md" paddingY="4rem">
      <Steps activeStep={activeStep} steps={steps} />

      <Text marginTop="2rem" textAlign="center">
        activeStep: {activeStep}
      </Text>
      <Box display="flex" justifyContent="space-around">
        <Button onClick={handleBack}>Back</Button>
        <Button onClick={handleNext}>Next</Button>
      </Box>
    </Container>
  )
}

export default Register

Steps.tsx

import { Box, Text } from "@chakra-ui/react"
import React from "react"
import StepLabel from "./StepLabel"

export interface Step {
  label: string
}

const Steps: React.VFC<{ activeStep: number; steps: Step[] }> = ({
  activeStep,
  steps,
}) => {
  const stepCount = steps.length
  return (
    <Box display="flex" flexDirection="row" justifyContent="center">
      {steps.map((step: Step, index: number) => {
        const isLastStep = index === stepCount - 1
        const isCurrentStep = index === activeStep
        const isCompletedStep = index < activeStep
        return (
          <>
            <StepLabel
              index={index}
              label={step.label}
              isCurrentStep={isCurrentStep}
              isCompletedStep={isCompletedStep}
              key={index}
            />
            <Text
              color={isCurrentStep || isCompletedStep ? "teal" : "gray"}
              marginX="1rem"
              marginY="auto"
              size="2rem"
              display={isLastStep ? "none" : ""}
            >
              →
            </Text>
          </>
        )
      })}
    </Box>
  )
}

export default Steps

StepLabel.tsx

import { CheckIcon } from "@/components/icons"
import { useColorModeValue } from "@chakra-ui/color-mode"
import { Box, Text } from "@chakra-ui/react"
import React, { useMemo } from "react"

const StepLabel: React.VFC<{
  index: number
  label: string
  isCurrentStep: boolean
  isCompletedStep: boolean
}> = ({ index, label, isCurrentStep, isCompletedStep }) => {
  const activeBg = "teal"
  const inactiveBg = useColorModeValue("gray.300", "gray.800")

  const getBgColor = useMemo(() => {
    if (isCompletedStep || isCurrentStep) return activeBg
    return inactiveBg
  }, [isCompletedStep, isCurrentStep, activeBg, inactiveBg])

  const getBorderColor = useMemo(() => {
    if (isCurrentStep || isCompletedStep) return activeBg
    return inactiveBg
  }, [isCurrentStep, isCompletedStep, activeBg, inactiveBg])
  return (
    <Box>
      <Box
        width="40px"
        height="40px"
        display="flex"
        justifyContent="center"
        alignItems="center"
        borderWidth="2px"
        borderRadius="50%"
        bgColor={getBgColor}
        borderColor={getBorderColor}
        color="white"
        marginX="auto"
        fontWeight="medium"
      >
        <Text display={isCompletedStep ? "none" : ""}>{index + 1}</Text>
        <Box display={!isCompletedStep ? "none" : ""}>
          <CheckIcon />
        </Box>
      </Box>
      <Text
        marginTop=".25rem"
        size="sm"
        color={isCurrentStep || isCompletedStep ? "teal" : "gray"}
      >
        {label}
      </Text>
    </Box>
  )
}

export default StepLabel

問題: その階層で知っている必要がない情報を保持している

以下で示しているように、各コンポーネント、ページの役割は赤枠で囲った範囲になります。
その下に知っておく必要がある情報を書いています

register	Steps	StepLabel

activeStep	activeStep, stepCount	stepNumber, stepLabel

activeStep: 現在のステップ
stepCount: ステップ数
stepNumber: ステップ番号(1,2)
stepLabel: ステップの名前（Step1,Step2)

しかし、beforeのコードでは必要以上の情報が上部(register, Steps)に記載されています。
関心の分離に基づくと、

register
- 現在のステップ
Steps
- 現在のステップ
- ステップの総数
StepLabel
- 表示する番号
- 表示するラベル
- 完了済みかどうか
- 現在表示しているステップかどうか

const steps: Step[] = [{ label: "Step 1" }, { label: "Step 2" }]

Steps.tsx

<StepLabel
  ...
  label={step.label}
  ...
/>

after

import { Box, Button, Text } from '@chakra-ui/react'
import { NextPage } from 'next'
import { useRouter } from 'next/router'
import { useState } from 'react'
import Layout from 'components/layout'
import { Seo } from '@/components/elements'
import { Step } from '@/presentationals/register/step'
import Steps from '@/presentationals/register/Steps'
import { scrollToTop } from '@/utils/animation/scrollToTop'

const Register: NextPage = () => {
  const router = useRouter()
  const steps: Step[] = ['Step 1', 'Step 2']

  const [activeStep, setActiveStep] = useState<Step>('basicInformation')
  const handleNext = () => {
    scrollToTop()
    setActiveStep('profile')
    router.push('/register?step2=true', undefined, { shallow: true })
  }
  const handleBack = () => {
    scrollToTop()
    setActiveStep('basicInformation')
    router.push('/register', undefined, { shallow: true })
  }

  return (
    <>
      <Seo title="会員登録" />
      <Container>
        <Steps activeStep={activeStep} steps={steps} />
        <Text marginTop="2rem" textAlign="center">
          activeStep: {activeStep}
        </Text>
        <Box display="flex" justifyContent="space-around">
          <Button onClick={handleBack}>Back</Button>
          <Button onClick={handleNext}>Next</Button>
        </Box>
      </Container>
    </>
  )
}

export default Register

Steps.tsx

import { Box, Text } from '@chakra-ui/react'
import React from 'react'
import { Step, step2Number } from './step'
import StepLabel from './StepLabel'

const Steps: React.VFC<{ activeStep: Step; steps: Step[] }> = ({
  activeStep,
  steps,
}) => {
  const activeStepNumber = step2Number(activeStep)
  const stepCount = steps.length
  return (
    <Box display="flex" flexDirection="row" justifyContent="center">
      {steps.map((step: Step) => {
        const isCurrentStep = step === activeStep
        const isCompletedStep = step2Number(step) < activeStepNumber
        const isLastStep = steps[stepCount - 1] === step
        return (
          <>
            <StepLabel
              isCurrentStep={isCurrentStep}
              isCompletedStep={isCompletedStep}
              key={step}
              step={step}
            />
            <Text
              color={isCurrentStep || isCompletedStep ? 'teal.500' : 'gray'}
              marginX="1rem"
              marginY="auto"
              size="2rem"
              display={isLastStep ? 'none' : ''}
            >
              →
            </Text>
          </>
        )
      })}
    </Box>
  )
}

export default Steps

StepLabel.tsx

import { useColorModeValue } from '@chakra-ui/color-mode'
import { Box, Text } from '@chakra-ui/react'
import React, { useMemo } from 'react'
import { Step, step2Label, step2Number } from './step'
import { CheckIcon } from '@/components/icons'

const StepLabel: React.VFC<{
  step: Step
  isCurrentStep: boolean
  isCompletedStep: boolean
}> = ({ step, isCurrentStep, isCompletedStep }) => {
  const stepNumber = step2Number(step)
  const stepLabel = step2Label(step)
  const activeBg = 'teal.500'
  const inactiveBg = useColorModeValue('gray.300', 'gray.800')

  const getBgColor = useMemo(() => {
    if (isCompletedStep || isCurrentStep) return activeBg
    return inactiveBg
  }, [isCompletedStep, isCurrentStep, activeBg, inactiveBg])

  const getBorderColor = useMemo(() => {
    if (isCurrentStep || isCompletedStep) return activeBg
    return inactiveBg
  }, [isCurrentStep, isCompletedStep, activeBg, inactiveBg])
  return (
    <Box>
      <Box
        width="40px"
        height="40px"
        display="flex"
        justifyContent="center"
        alignItems="center"
        borderWidth="2px"
        borderRadius="50%"
        bgColor={getBgColor}
        borderColor={getBorderColor}
        color="white"
        marginX="auto"
        fontWeight="medium"
      >
        <Text display={isCompletedStep ? 'none' : ''}>{stepNumber}</Text>
        <Box display={!isCompletedStep ? 'none' : ''}>
          <CheckIcon />
        </Box>
      </Box>
      <Text
        marginTop=".25rem"
        size="sm"
        color={isCurrentStep || isCompletedStep ? 'teal.500' : 'gray'}
      >
        {stepLabel}
      </Text>
    </Box>
  )
}

export default StepLabel

まとめ

そのコンポーネントが何を知っているか何をするものかをきちんと明確にした上でコードを書くことが重要なんだなと感じました。

関心の分離以外にも指摘箇所はあったのですが、修正する前後で可読性がグッと上がったのでこの考えを頭に入れながら今後も開発を進めていこうと思います。

もさっとしていたコードがスッキリしたのでとても気分が良いです。
ではまた！

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up