10분 TV

# 코드리뷰 (2022.10.04)

# Vue.js

# 다른 객체에게 일을 시킬 때 판단요소도 위임하자

  • 다른 객체에게 무엇인가 요구하면서 묻는 행위 금지
  • 일을 시켰으면 업무 처리 방식에 대해서는 관심을 두지 말자

# as is

onBatchMenu(index) {
    if (selectedCount == 1) {
        switch (index) {
            case 0:
                updateRewardState.show(selectedCount, state);
                break;
            case 1:
                updateRewardExpectedAt.show(selectedCount, rewardExpectedAt);
                break;
            case 2:
                updateRewardPoint.show(selectedCount, point);
                break;
        }
        return
    }

    switch (index) {
        case 0:
            updateRewardState.show(selectedCount);
            break;
        case 1:
            updateRewardExpectedAt.show(selectedCount);
            break;
        case 2:
            updateRewardPoint.show(selectedCount);
            break;
    }
},
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28

# to be

onBatchMenu(index) {
    switch (index) {
        case 0:
            updateRewardState.show(selectedCount, state);
            break;
        case 1:
            updateRewardExpectedAt.show(selectedCount, rewardExpectedAt);
            break;
        case 2:
            updateRewardPoint.show(selectedCount, point);
            break;
    }
},
1
2
3
4
5
6
7
8
9
10
11
12
13

# 숫자 대신 의미있는 이름으로 변경

async upload2(path, blob, name) {
    ...
},

data() {
    return {
        columnManager1: new QuasarColumnManager(columns1),
        columnManager2: new QuasarColumnManager(columns2),
        columns1: columns1,
        columns2: columns2,
        rows1: this.userDetail.pointHistory,
        rows2: [],
    }
},
1
2
3
4
5
6
7
8
9
10
11
12
13
14

# 코드(codes.js)로 분리해서 관리

data() {
    return {
        influencerGrades: [
            { label: "Celeb", value: 0 },
            { label: "Macro", value: 1 },
            { label: "Micro", value: 2 },
            { label: "Reviewer", value: 3 },
        ],

        steps: [
            { label: "섭외", value: 0 },
            { label: "진행", value: 1 },
            { label: "게재", value: 2 },
            { label: "완료", value: 3 },
        ],

        states: [
            { label: '지원완료', value: 0 },
            { label: '초대 중', value: 1 },
            { label: '거절', value: 2 },
            { label: '미게재', value: 3 },
            { label: '확인중', value: 4 },
            { label: '확인 요청', value: 5 },
            { label: '미승인', value: 6 },
            { label: '진행취소', value: 7 },
            { label: '완료', value: 7 },
        ],
    }
},
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
  • codes.js를 통해서 캡슐화하고 중복 사용 방어
  • 서버로부터 새로운 코드가 자동으로 갱신되도록

# 코드 단순화

# as is

methods: {
    getColumnHeader(col) {
        return this.columnManager.getHeader(col);
    },

    getColumn(col) {
        return this.columnManager.getColumn(col);
    },

    getCode(col) {
        return this.columnManager.getCode(col);
    },
}
1
2
3
4
5
6
7
8
9
10
11
12
13

# to be

methods: {
    ... this.columnManager,
}
1
2
3

# 템플릿 영역에서 자주 사용하는 외부 메소드를 VueBase에 추가

methods: {
    toLink(text) {
        return strg.toLink(text);
    },
}
1
2
3
4
5
  • VueBase 추가 또는 spread operator(...)를 사용해서 methods에 추가한다. (columnManager 동일)

# 객체 속성이 너무 많을 때 초기화 가이드

# as is

state: () => ({
    ...
    params: {
        title: null,
        email: null,
        brand: null,
        category: null,
        platform: null,
        status: null,
        createdFrom: null,
        createdTo: null,
        applicationDeadlineFrom: null,
        applicationDeadlineTo: null,
    },
}),
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

# to be

state: () => ({
    ...
    params: { },
}),
1
2
3
4
  • undefinded가 문제되지 않는 경우에 굳이 null 값을 갖는 속성을 초기화할 필요가 없다.

# 코드 일관성 이슈

# as is

if (campaignQuestion.isRequire) {
    campaignQuestion.isRequire = 1;
} else campaignQuestion.isRequire = 0;
1
2
3

# to be

if (campaignQuestion.isRequire) {
    campaignQuestion.isRequire = 1;
} else {
    campaignQuestion.isRequire = 0;
}
1
2
3
4
5

# Flutter

# 상속 상태에서 초기화 및 종료처리 순서

# as is

@override
void initState() {
    defaultTextStyle = TextStyle(color: ...);
    super.initState();
}

@override
void dispose() {
    super.dispose();
    controller.dispose();
}
1
2
3
4
5
6
7
8
9
10
11

# to be

@override
void initState() {
    super.initState();
    defaultTextStyle = TextStyle(color: ...);
}

@override
void dispose() {
    controller.dispose();
    super.dispose();
}
1
2
3
4
5
6
7
8
9
10
11
  • 상속 받아서 사용하는 리소스가 준비가 되지 않은 상태에서 초기화가 이루어지면서 에러가 발생할 가능성이 있다.
  • 종료처리의 경우도 같은 오류가 발생할 가능성이 있다.
  • 당장은 문제가 없더라도 안전한 습관을 길들이는 것이 좋다.

# null check issue #1

# as is

String? brandName;
if (_campaign.brand != null &&
    _campaign.brand?.brand_name.isNotEmpty) {
    brandName = _campaign.brand?.brand_name as String;
} else {
    brandName = "";
}
1
2
3
4
5
6
7

# to be

String brandName = _campaign.brand?.brand_name ?? "";
1

# null check issue #2

# as is

{
    ...
    itemCount: point == null ? 0 : point!.length,
    ...
}
1
2
3
4
5

# to be

{
    ...
    itemCount: point?.length ?? 0,
    ...
}
1
2
3
4
5

# 복잡한 분기문을 단순하게

# as is

Future<void> changeEmail(String? inputEmail) async {
    this.inputEmail = inputEmail;
    if (inputEmail!.isNotEmpty) {
        bool isValidEmail = isEmail(inputEmail!);
        if (inputEmail == loginUser!.email) {
            validEmail = isValidEmail;
            return;
        }
        if (isValidEmail) {
            busy();
            final checkResult = await checkEmail(inputEmail);
            validEmail = checkResult.result;
            emailMessage = checkResult.message;
            showEmailMessage = true;
        }
    } else {
        validEmail = false;
        emailMessage = "";
        showEmailMessage = false;
    }
    success();
}
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22

# to be

Future<void> changeEmail(String inputEmail) async {
    this.inputEmail = inputEmail;

    validEmail = false;
    emailMessage = "";
    showEmailMessage = false;

    if (inputEmail.isEmpty) return;

    bool isValidEmail = isEmail(inputEmail);
    if (isValidEmail == false) return;

    validEmail = (inputEmail == loginUser?.email);
    if (validEmail) return;

    busy();
    try {
        final checkResult = await checkEmail(inputEmail);
        validEmail = checkResult.result;
        emailMessage = checkResult.message;
        showEmailMessage = true;
    } finally {
        success();
    }
}
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25

# 외부의 영향을 최소화하기 외 기타

# as is

Future<void> saveUrl() async {
    if (this.inputPostingUrl!.isEmpty) {
        CommonUtils.toast("URL을 입력해 주세요.");
        return;
    }

    this.busy();
    if (this.inputPostingUrl != null) {
        final result = await _campaignService.saveUrlToApplyCampaign(...);
        if (result) {
            await this.fetchApplyData(this.campaignApply!.campaign!.id);
            CommonUtils.toast("정상적으로 등록 되었습니다.");
        } else {
            CommonUtils.toast("URL 등록시 에러가 발생했습니다. 다시 시도해 주세요.");
        }
    }
    this.idle();
}
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

# to be #1

Future<void> saveUrl() async {
    if (inputPostingUrl?.isEmpty ?? true) {
        CommonUtils.toast("URL을 입력해 주세요.");
        return;
    }

    busy();
    try {
        final result = await _campaignService.saveUrlToApplyCampaign(...);
        if (result) {
            fetchApplyData(campaignApply!.campaign!.id);
            CommonUtils.toast("정상적으로 등록 되었습니다.");
        } else {
            CommonUtils.toast("URL 등록시 에러가 발생했습니다. 다시 시도해 주세요.");
        }
    } finally {
        idle();
    }
}
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
  • 2: 반복되는 null check issue

# to be #2

Future<void> saveUrl(int? campaignId, String? url) async {
    if (campaignId == null) return;
    if (url == null) return;

    if (url.isEmpty) {
        CommonUtils.toast("URL을 입력해 주세요.");
        return;
    }

    busy();
    try {
        final result = await _campaignService.saveUrlToApplyCampaign(url);
        if (result) {
            fetchApplyData(campaignId);
            CommonUtils.toast("정상적으로 등록 되었습니다.");
        } else {
            CommonUtils.toast(result.errorMsg);
        }
    } finally {
        idle();
    }
}
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
  • 2-3: 지역 변수의 경우 null check 이후에는 ! 또는 ? 기호를 사용하지 않아도 된다.
  • 17: 에러 메시지가 발생하는 원인을 클라이언트에서는 알 수 없으므로 서버에서 메시지를 보내주는 것으로 변경한다.