1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@doc_lab

AIが書いたGoのコードを、Javaエンジニアが本気でリファクタリングしてみた

1
Posted at

はじめに

最近はずっと 個人開発 に関する記事を書いています。
今回も 個人開発 に関する事なんですが、今までと少し違いリファクタリングについての記事を書きたいと思います。

AIエージェントが大量に書いたコードを棚卸して、可読性を上げる または 拡張性を持たせるを軸にリファクタリングしてみました。

やってみての感想

まず先に感想をお伝えします。

実務が３年未満とあまり長くないので
最初は以下の３点で不安を抱えていました。

  • 他人のソースコードレビューも実務でやったことない
  • Java と Python しか実装経験ないけどGoなんて見てわかるのか
  • 今の状態でリファクタリングなんてできるのか

現実的に 「Kiro を使った個人開発でGo言語を扱ってみた！」
という少し無茶ぶり感があったので、ある意味 大きな挑戦！ でした。

でもやってみると意外と楽しい！し、わかるようになる。
それが私の感想です。

例えば下記のようなソースだと、空文字のチェックをしています。

	if req.Email == "" || req.Password == "" {
		return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", "email and password are required"))
	}

実務でよく見るisEmpty()メソッドがすぐ頭に出てきたので、すかさずcommon.goを作成し共通的な部品として切り出しました。
そうする事で別のクラスでも再利用できます。
等々。。。

初歩的な事かも知れませんが学びも多かったです。
その詳細を次から書いていこうと思います。

再利用できる共通部品を作る

この部品の詳細は、上のやってみての感想セクションで触れています。

まず修正前のソースコードです。
先程も参照していますが、この空文字チェックをcommon.goに共通部品として作りました。

修正前

backend/handler/auth.go
	if req.Email == "" || req.Password == "" {
		return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", "email and password are required"))
	}

修正後

backend/handler/auth.go
import "github.com/bus-logistics/backend/utils"

	// 入力値チェック
	if utils.IsEmpty(req.Email) || utils.IsEmpty(req.Password) || utils.IsEmpty(req.Role) {
		return c.JSON(http.StatusUnprocessableEntity, errResponse("VALIDATION_ERROR", "missing required fields"))
	}
backend/utils/common.go
import "reflect"

/*
 IsEmpty は任意の値が「空」であるかを判定する。空の定義は以下の通り：
 - nil
 - ゼロ値（数値なら0、文字列なら空文字、スライスやマップなら長さ0など）
*/
func IsEmpty(any interface{}) bool {
	if any == nil {
		return true
	}

	v := reflect.ValueOf(any)

	switch v.Kind() {
	case reflect.String, reflect.Array, reflect.Slice, reflect.Map:
		return v.Len() == 0
	case reflect.Ptr, reflect.Interface:
		return v.IsNil()
	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
		return v.Int() == 0
	case reflect.Float32, reflect.Float64:
		return v.Float() == 0
	case reflect.Struct:
		return reflect.DeepEqual(any, reflect.New(v.Type()).Elem().Interface())
	default:
		return false
	}
}

型の安全性: interface{}（Any型）と reflect（リフレクション）を使うと、Javaの Object を使った汎用メソッドに近い柔軟性が得られますが、実行速度が少し落ちたり、実行時エラーのリスクが増えたりします。
※上記は１つのデメリットです。

IsEmpty()メソッドで色んな型のチェックをしているのが気になりますが、今後もし不都合が出てきたら処理を分割予定です。
今はそのまま使用しています。

今回は、私自身の可読性を上げたくてあえてこの選択をしました。

if文をswitchにしてみる

エラーのハンドリングがif文で同じような処理を複製していたのでswitchでスッキリさせました。

修正前

backend/handler/schedule.go
	if err != nil {
		if errors.Is(err, service.ErrOriginRequired) {
			return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", err.Error()))
		}
		if errors.Is(err, service.ErrDestRequired) {
			return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", err.Error()))
		}
		if errors.Is(err, service.ErrDepartAtPast) {
			return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", err.Error()))
		}
		if errors.Is(err, service.ErrInvalidMaxWeight) {
			return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", err.Error()))
		}
		if errors.Is(err, service.ErrInvalidMaxSize) {
			return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", err.Error()))
		}
		return c.JSON(http.StatusInternalServerError, errResponse("INTERNAL_ERROR", "internal server error"))
	}

	return c.JSON(http.StatusCreated, scheduleToMap(*schedule, false))

修正後

backend/handler/schedule.go
	if err != nil {
		switch {
		// バリデーションエラーの場合は400 Bad Requestを返す
		case errors.Is(err, service.ErrOriginRequired),
			 errors.Is(err, service.ErrDestRequired),
			 errors.Is(err, service.ErrDepartAtPast),
			 errors.Is(err, service.ErrInvalidMaxWeight),
			 errors.Is(err, service.ErrInvalidMaxSize):
			return c.JSON(http.StatusBadRequest, errResponse("VALIDATION_ERROR", err.Error()))
		// 認証エラーの場合は401 Unauthorizedを返す
		default:
			return c.JSON(http.StatusInternalServerError, errResponse("INTERNAL_ERROR", "internal server error"))
		}
	}
	return c.JSON(http.StatusCreated, scheduleToMap(*schedule, false))

このリファクタリングで５行程、ソースコードが省略されました！

エラーハンドリング後のreturnを簡素化

エラーハンドリングのreturnが長かったので共通化してみました。
これで見た目もスッキリしたような気がします。

修正前

schedule.go
	userIDStr, ok := c.Get("user_id").(string)
	if !ok {
		return c.JSON(http.StatusUnauthorized, errResponse("UNAUTHORIZED", "missing user_id"))
	}

修正後

schedule.go
import "github.com/bus-logistics/backend/utils"
	userIDStr, ok := c.Get("user_id").(string)
	if !ok {
		return utils.NewAppError(http.StatusUnauthorized, "UNAUTHORIZED", "missing user_id")
	}

ちなみにerrors.goはこんな感じです。

errors.go
package utils

type AppError struct {
    StatusCode int    // HTTPステータスコード
    ErrorCode  string // "VALIDATION_ERROR" などの識別子
    Message    string // ユーザー向けメッセージ
}

// error インターフェースを満たすためのメソッド
func (e *AppError) Error() string {
    return e.Message
}

// 頻出するエラーを楽に作るためのヘルパー
func NewAppError(status int, code string, msg string) *AppError {
    return &AppError{status, code, msg}
}

おわりに

いくつか気付いた点を列挙させて下さい。

  • GoのinterfaceはJavaのimplements不要のポリモーフィズムだ！と感じました
    AppErrorにError()メソッドを実装しただけで、標準のerror型として扱えるようになった

  • リフレクションの代償
    IsEmptyでリフレクションを使ったことに対し、JavaのObjectっぽく扱える魔法だけど、Goでは用法用量を守って使おう（Goでは推奨されていない使い方みたいですね）

  • AIとの共存
    AIは大量にコードを書いてくれるが、構造を作るのは人間のエンジニアの仕事ですね

個人開発 を通して思ったより色々な事を学べました！
以前は「転職活動を有利に進めたい」という思いで投稿してましたが、、、

現在は素直に個人開発を楽しんでます！
この後も引き続き楽しんでいきたいと思うので、応援よろしくお願いします。

1
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?