V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
mascteen
V2EX  ›  程序员

这是一个面试项目,赐教还有哪些需要优化的地方?

  •  1
     
  •   mascteen · 2020-12-16 10:00:12 +08:00 · 2346 次点击
    这是一个创建于 398 天前的主题,其中的信息可能已经有所发展或是发生改变。

    Table of Contents

    1. 这是一个面试项目,赐教还有哪些需要优化的地方?
    2. 项目需求
    3. 我的代码

    这是一个面试项目,赐教还有哪些需要优化的地方?

    这是一个公司的面试小项目,根据数据生成多选框,要有一个全选的选项目,这是我的代码,请问还有哪些需要改进的地方和不足?因为对方不回复,只能来这里求教。

    项目需求

    1. 写一个多选框小组件;
    2. 需要有单元测试
    3. 有全选选项目
    4. 样式符合需求
    5. 用 react,typescript,jest 技术
    6. 全程不能出现中文
    7. 需要有意义的注释
    8. 代码尽量优美
    9. 取名优美
    10. 最后提交 PR 审核

    我的代码

    • 多选框组件 MultiCheck.tsx

          import React, { useState, useEffect, FunctionComponent } from "react";
          import { fpMap, pipe } from "../utils";
          import "./MultiCheck.css";
      
          export type Option = {
          label: string;
          value: string;
          checked: boolean;
          };
      
      /**
       * Notice:
       * 1. There should be a special `Select All` option with checkbox to control all passing options
       * 2. If columns > 1, the options should be placed from top to bottom in each column
       *
       * @param {string} label - the label text of this component
       * @param {Option[]} options - options
       * @param {number} columns - default value is 1
       * @param {Function} onChange - when checked options are changed,
       *                             they should be passed to outside
       */
      type Props = {
        label?: string;
        options: Option[];
        columns?: number;
        onChange?: (options: Option[]) => void;
      };
      
      const selectAll = (c = false) => ({
        label: "Select All",
        value: "Select All",
        checked: c
      });
      
      const MultiCheck: FunctionComponent<Props> = (props): JSX.Element => {
        const { label = "", columns = 1, onChange = () => {} } = props;
        const [value, setValue] = useState({}); // for call props.onChange
        const [options, setOptions] = useState<Option[]>([
          /* if original options all checked */
          selectAll(!hasUnchecked(props.options)),
          ...props.options
        ]);
      
        function hasUnchecked(o: Option[]) {
          return o.filter(o => o.value !== selectAll().value).some(o => !o.checked);
        }
      
        function _handleClick(o: Option): void {
          /* user toggle click checkbox */
          const convertCheck = fpMap(i => {
            if (i.value === o.value) {
              return { ...i, checked: !o.checked };
            }
            return i;
          });
          /* user toggle click SelectAll option */
          const convertSelectAll = fpMap(i => {
            if (selectAll().value === o.value) {
              return { ...i, checked: !o.checked };
            }
            return i;
          });
          /* if all other option checked than SelectAll option checked else unchecked */
          const isAllChecked = (o: Option[]) => {
            return fpMap(i => {
              if (i.value === selectAll().value) {
                return { ...i, checked: !hasUnchecked(o) };
              }
              return i;
            })(o);
          };
      
          setOptions(prev =>
            pipe(convertCheck, convertSelectAll, isAllChecked)(prev)
          );
          setValue(o);
        }
      
        function _getColumns(n: number): number {
          return n > 0 ? Math.ceil(options.length / n) : 1;
        }
      
        useEffect(() => {
          /* selectAll option can not be pass outside */
          onChange(options.filter(i => i.value !== selectAll().value));
        }, [value]);
      
        return (
          <div className="multi-check-container">
            <div className="multi-check">
              <div className="multi-check-label">
                <label>{label}</label>
              </div>
              <div
                className="multi-check-items"
                style={{
                  gridTemplateRows: `repeat(${_getColumns(columns)},auto)`
                }}
              >
                {fpMap(o => (
                  <label className="multi-check-item" key={o.value}>
                    <input
                      type="checkbox"
                      value={o.value}
                      checked={o.checked}
                      onChange={() => _handleClick(o)}
                    />
                    <span></span>
                    <div>{o.label}</div>
                  </label>
                ))(options)}
              </div>
            </div>
          </div>
        );
      };
      
      export default MultiCheck;
      
    • 测试用例

          import "@testing-library/jest-dom";
          import React from "react";
          import renderer from "react-test-renderer";
          import { fireEvent, render, screen } from "@testing-library/react";
          import { fpMap, pipe } from "../utils";
          import MultiCheck from "./MultiCheck";
      
      describe("MultiCheck", () => {
        describe("initialize", () => {
          it("renders correctly", () => {
            const tree = renderer.create(<MultiCheck options={[]} />).toJSON();
      
            expect(tree).toMatchSnapshot();
          });
      
          it("renders the label if label provided", () => {
            const label = "jest test";
            render(<MultiCheck options={[]} label={label} />);
      
            expect(screen.queryByLabelText(label)).toBeDefined();
          });
      
          it("render and click checkbox", () => {
            const options = [{ label: "test", value: "test", checked: false }];
            render(<MultiCheck options={options} />);
      
            expect(screen.queryByLabelText(/test/i)).not.toBeChecked();
      
            // simulate user click checkbox
            fireEvent.click(screen.getByLabelText(/test/i));
      
            expect(screen.queryByLabelText(/test/i)).toBeChecked();
          });
        });
      
        describe("utils", () => {
          it("fpMap", () => {
            let arr = [1, 2, 3];
            let add1 = (o: number) => o + 1;
      
            expect(fpMap(add1)(arr)).toEqual([2, 3, 4]);
          });
      
          it("pipe", () => {
            let arr = [1, 2, 3];
            let add1Each = (i: []) => i.map(o => o + 1);
            let prod2Each = (i: []) => i.map(o => o * 2);
      
            expect(pipe(add1Each, prod2Each)(arr)).toEqual([4, 6, 8]);
          });
        });
      });
      
    • 工具代码 utils.ts

          type fn = (arg: any) => any;
      
      // functional programming for map iterator
      export function fpMap(func: fn): fn {
        return function(arr: unknown[]): unknown[] {
          let length: number = arr.length || 0;
          let i: number = 0;
          let result: unknown[] = [];
      
          while (i < length) {
            result.push(func(arr[i]));
            i++;
          }
          return result;
        };
      }
      
      // functional programming pipe
      export function pipe(...fns: fn[]): fn {
        return function(x: any) {
          return fns.reduce((v: any, f: fn) => f(v), x);
        };
      }
      
    12 条回复    2020-12-16 19:09:59 +08:00
    mascteen
        1
    mascteen  
    OP
       2020-12-16 11:38:10 +08:00
    很完美么?
    tesguest123
        2
    tesguest123  
       2020-12-16 11:50:26 +08:00 via iPhone
    交完后,你不适合我们公司 doge 。白嫖一波
    mascteen
        3
    mascteen  
    OP
       2020-12-16 11:52:22 +08:00 via Android
    @tesguest123 对呀,所以来这里问问
    aaronlam
        4
    aaronlam  
       2020-12-16 14:51:16 +08:00
    这是个现场上机题?
    Chrisssss
        5
    Chrisssss  
       2020-12-16 17:54:06 +08:00
    比如 `_getColumns` 不关乎业务的东西可以提到 FC 外面去,部分 any 定义使用泛型,有一些代码风格不太好,比如 `selectAll` 函数的 c 参数命名,其实可以用 `checked`,然后里面直接`{
    label: "Select All",
    value: "Select All",
    checked
    }` 就好。`‘Select All’` 可以定义一个常量。这些就是我作为面试官的话看了一眼会给你扣分的地方🐶
    Chrisssss
        6
    Chrisssss  
       2020-12-16 17:54:38 +08:00
    其实还有很多问题😢
    buhi
        7
    buhi  
       2020-12-16 17:59:26 +08:00
    好好的 Array#map 不用, 造了个 any=>any=>(any[])=>any[], 一通下来全给你整 any 了
    这就是你认为的函数式编程吗? 遇见我同事这么做直接打死了
    mascteen
        8
    mascteen  
    OP
       2020-12-16 18:41:33 +08:00 via Android
    @aaronlam 不是,github
    mascteen
        9
    mascteen  
    OP
       2020-12-16 18:44:41 +08:00 via Android
    @buhi 大哥这能一样么?
    mascteen
        10
    mascteen  
    OP
       2020-12-16 18:58:52 +08:00 via Android
    @Chrisssss 没时间打字?
    mascteen
        11
    mascteen  
    OP
       2020-12-16 19:04:31 +08:00 via Android
    @Chrisssss 是有一些不足,不过这是一道面试题,我不会花太多时间在上面的
    yzbythesea
        12
    yzbythesea  
       2020-12-16 19:09:59 +08:00
    逻辑挺简单的,但是我就是读不懂你的代码。。。

    为什么不用 array 存 option ?单独造一个 selectAll 的类干什么?为什么不把 selectAll 当作一个 option 加进去呢?

    然后 checkbox 的逻辑都可以但写出 function,再套进里面,你这么写看着好晕。。。
    关于   ·   帮助文档   ·   API   ·   FAQ   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   1767 人在线   最高记录 5497   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 23ms · UTC 17:10 · PVG 01:10 · LAX 09:10 · JFK 12:10
    ♥ Do have faith in what you're doing.