code climateのsmellチェックが厳しすぎる件

モチベーション

GithubのREADMEにバッジがあるだけでそれっぽく見える

code climateはコードの不吉な臭い(code smell)とかtest coverageを管理してくれるツール

codeclimate.com

今やってるbotのリポジトリと連携させてるけどめちゃくちゃチェックが厳しい

Issue

code climateでissueが3個発見された

issueってのはお前のコードくせえわってやつ

発見されたのは3つだけども、大別すると2つだった

1 functionあたりの行数多すぎて臭い

NewLineClient has 31 lines of code (exceeds 25 allowed). Consider refactoring. こんな表示がされる。

1 functionあたりのコード行は25行以内が推奨されているらしい。これを意地でも守ろうとすると結構辛くないか？ 25行以内であれば何個functionがあってもいいよって考えなんだろうか？特にポリシーをみたわけじゃないから邪推しかできない。

func NewLineClient() LineClient {
    values := url.Values{}
    values.Set("grant_type", "client_credentials")
    values.Set("client_id", os.Getenv("CHANNEL_ID"))
    values.Set("client_secret", os.Getenv("CHANNEL_SECRET"))
 
    url := "https://api.line.me/v2/oauth/accessToken"
 
    req, err := http.NewRequest(
        "POST",
        url,
        strings.NewReader(values.Encode()),
    )
    if err != nil {
        log.Fatal(err)
    }
 
    req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 
    client := &http.Client{}
    res, err := client.Do(req)
    if err != nil {
        log.Fatal(err)
    }
    defer res.Body.Close()
 
    authConfig := new(lineClient)
 
    if err := json.NewDecoder(res.Body).Decode(authConfig); err != nil {
        log.Fatal(err)
    }
 
    client := &lineClient{
        AccessToken:   authConfig.AccessToken,
        ExpiresIn:     authConfig.ExpiresIn,
        TokenType:     authConfig.TokenType,
        ChannelSecret: os.Getenv("CHANNEL_SECRET"),
    }
 
    return client
}

引っかかったのはこのfunction。空行は含めずに31行あるみたいだ。

あと6行削るとなるとどこかを別のfuncに切り出すか、lineClientのフィールドをAuthConfigとChannelSecretの2つにしてちょいとスタイリッシュにするかが考えられると後者だとぎりぎり足りないな。

ただ、前者にしても問題は発生する。それが次の警告だ

1functionで参照しすぎて臭い

Method lineReminder.lineReminder.lineReminder.GetWebHook has a Cognitive Complexity of 9 (exceeds 5 allowed).

1functionあたり、参照する回数は5回以下が望ましいらしい。

func (l *lineReminder) GetWebHook(req *http.Request) (string, error) {
    received, err := l.client.ReceiveEvent(req)
    if err != nil {
        return "", err
    }

    var status = "false"

    for _, event := range received {
        //log.Println("groupId: " + event.Source.GroupID)
        //log.Println("userId: " + event.Source.UserID)
        textMsg := new(linebot.TextMessage)
        byteMsg, _ := event.Message.MarshalJSON()
        if err := json.Unmarshal(byteMsg, textMsg); err != nil {
            //画像メッセージの場合もあるからただエラーを出力するだけにする
            log.Println(err.Error())
        }

        if textMsg.Text == os.Getenv("REPORT_MESSAGE") {
            status = SetStatus(event.Source.UserID, "true")
            err := l.client.ReplyMessage(event.ReplyToken, os.Getenv("REPLY_SUCCESS"))
            if err != nil {
                return "", err
            }
        }
    }

    return status, nil
}

うん。確かにこのfunctionは臭いな。 lineに投稿があった際に飛ぶwebhookを受け取ってeventを取得するfuntionなんだけども、messageを抽出する部分が臭いを発している気がする。

変更してみた

func (l *lineReminder) GetWebHook(req *http.Request) (string, error) {
    received, err := l.client.ReceiveEvent(req)
    if err != nil {
        return "", err
    }

    var status = "false"

    for _, event := range received {
        //log.Println("groupId: " + event.Source.GroupID)
        //log.Println("userId: " + event.Source.UserID)
        msg, _ := ExtractMessage(event)
        if msg == os.Getenv("REPORT_MESSAGE") {
            status = SetStatus(event.Source.UserID, "true")
            err := l.client.ReplyMessage(event.ReplyToken, os.Getenv("REPLY_SUCCESS"))
            if err != nil {
                return "", err
            }
        }
    }

    return status, nil
}

func ExtractMessage(event linebot.Event) (string, error) {
    textMsg := new(linebot.TextMessage)
    byteMsg, _ := event.Message.MarshalJSON()
    if err := json.Unmarshal(byteMsg, textMsg); err != nil {
        return "", err
    }
    return textMsg.Text, nil
}

Method lineReminder.lineReminder.lineReminder.GetWebHook has a Cognitive Complexity of 7 (exceeds 5 allowed).

デスヨネ。でもこれ以上function切ると可読性が著しく落ちる気がする。眠いからもうやめた

おわりに

一応MaintainabilityはAだよ