A Line-by-Line Code Review Fixes Variable Flow and Init Order in a Go CAPTCHA Module
This is a series blog. The author, as a PHP full-stack engineer, will use AI tools (Claude Code, Codex, DeepSeek, Doubao, etc.) to learn the Go language from scratch and ultimately complete the open-source project ai-go-mall (github | gitee), documenting and sharing the entire process.
In the previous installment, we completed the "first version of the click verification code and encapsulated random number and file system packages." This installment will complete: a line-by-line visual inspection of the click verification code.
Line-by-Line Visual Inspection of Click Verification Code
Naming Detail Changes
The following structure works, but ImageBase64 has an Image prefix, while Width and Height, which also represent the final image dimensions, do not. Let the AI add the prefix.
// ClickResult is the return result for creating a verification code
type ClickResult struct {
Key string `json:"key"`
Width int `json:"width"`
Height int `json:"height"`
Elements []string `json:"elements"`
ImageBase64 string `json:"image_base64"`
}
Encapsulatable Functions
There is a readDir function used to read background images and icons. It can be migrated to the pkg/filesystem package and modified to support recursive reading of subdirectories, obtaining a list of all files.
func readDir(dir string) ([]string, error) {
entries, err := os.ReadDir(dir)
if err != nil {
return nil, err
}
var paths []string
for _, e := range entries {
if !e.IsDir() {
paths = append(paths, filepath.Join(dir, e.Name()))
}
}
return paths, nil
}
Variable Abnormal Flow
The following extracted code has an iconPaths variable. After being read inside the Create function, it is passed around and used in multiple places.
iconPaths, err := readDir(cfg.IconDir)
correctElements := generateElements(cfg.Length, cfg.Elements, iconPaths)
confusionElements := generateElements(cfg.ConfusionLength, cfg.Elements, iconPaths)
info, rect, drawErr := drawElement(bg, elem, bgW, bgH, iconPaths, placedRects)
drawIcon(bg, elem, bgW, bgH, iconPaths, placed)
- Change the verification code configuration file (where the icon path is configured) to declare an
iconsarray. The array stores theiconfile name, Chinese name (translated from the file name), and file path. Read it globally and use it without passing it around everywhere. - If the
Elementsoutput by theCreatefunction isicon, it should use the Chinese name, wrapped in<>, e.g.,<Icon Chinese Name>,<Chinese Character>,<Uppercase Letter>.
I have a feeling these few modifications will be quite large. Let it do them first, and then review again later:
All done. Summary of changes for the four issues...
Configuration File Readability Optimization
In this round, the icon data was placed in the configuration file, but it doesn't look very good:
# Icon Metadata
icons:
- name: "aeroplane"
cn_name: "飞机"
- name: "apple"
cn_name: "苹果"
Let it use the file name as en_name and the Chinese name as cn_name, while retaining the possibility of adding name for other languages in the future:
icons:
- en_name: "aeroplane"
cn_name: "飞机"
- en_name: "apple"
cn_name: "苹果"
- en_name: "banana"
cn_name: "香蕉"
# ......
It looks slightly better, but still, placing icons at the very bottom of the configuration file would be a bit better.
Code Readability Optimization
In the current internal\infra\captcha\click.go file, the Create function first calls the clean expired verification codes and icon initialization functions.
Then, further down in the file, the init function initializes the Chinese character pool, roughly like this:
pool := "的一是在不了有和人这中大为上个国我以要他..."
chineseRunes = []rune(pool)
It's like there is an init, but clean expired verification codes and icon initialization are placed in Create to execute. This might be because some previous prompt of mine misled it. Putting icon initialization in Create is barely understandable, but clean expired verification codes is actually more reasonable to put in init. Here, let's just keep it uniform: put all initialization-related actions into init. Create will no longer initialize certain necessary data separately but will use them directly, focusing on its own job.
Also, move the init function to the very top of the file to improve human readability (PS: init is a special function in the Go language. When a package is imported, the package's init is automatically executed. If there are multiple init functions, they execute in order).
When I asked the AI to modify it, it told me there was a reason for doing it this way:
When init() executes, config.Init() and database.Init() have not yet run, but icons depend on config, and cleanExpired depends on the database, so they cannot be put into init().
The solution closest to your intent: use a single sync.Once bootstrap function to unify all three, and Create only needs one line of calling.
The AI means that not all initialization work can be completed during init because config.Init() has not been called yet. Then it implemented a bootstrap function, called once at the beginning of Create. init still retained the function of initializing the Chinese character pool, which is equivalent to still having two initialization functions. Here, let's just remove init entirely and put all initialization work into the bootstrap function. Internally, it's divided into two parts: tasks that only need to be executed once go into sync.Once, and clean expired verification codes, which needs to be executed repeatedly, cannot go into sync.Once.
Since there is a unified global initialization function, let the AI also extract public variables like cfg in cfg := config.Get().Captcha into global variables, so they don't need to be assigned individually everywhere.
Understanding the Code
A visual inspection of the code shows no structural problems, no redundant encapsulation, overall clean and concise, with smooth and clear logic. However, I also cannot understand much of the code, for example:
func loadImage(path string) (*image.RGBA, error) {
f, err := os.Open(path)
if err != nil {
return nil, err
}
defer f.Close()
img, err := png.Decode(f)
if err != nil {
return nil, err
}
rgba := image.NewRGBA(img.Bounds())
draw.Draw(rgba, rgba.Bounds(), img, img.Bounds().Min, draw.Src)
return rgba, nil
}
This is a function for loading a background image. It uses things like image.NewRGBA, img.Bounds(), draw.Draw that I have never seen before. I'll just copy it out here and let Doubao explain it to me.
Our goal is to use AI to learn the Go language. Encountering code we don't understand is exactly the time to learn. Moreover, as developers, we ultimately need to achieve an understanding of all the code in the entire file: what was done first, what was done later, how it was done, and even make some minor adjustments or personalized changes. First understand, then manually fine-tune:
img, err := png.Decode(f)
if err != nil {
return nil, err
}
// Changed to
img, _, err := image.Decode(f)
if err != nil {
return nil, err
}
// ==============================================
rgba := image.NewRGBA(img.Bounds())
draw.Draw(rgba, rgba.Bounds(), img, img.Bounds().Min, draw.Src)
return rgba, nil
// Changed to
bounds := img.Bounds()
rgba := image.NewRGBA(bounds)
draw.Draw(rgba, rgba.Bounds(), img, bounds.Min, draw.Src)
return rgba, nil
Top 1 of 2 from juejin.cn, machine-translated. The original thread is authoritative.
handler -> service -> repository -> model, same style as when I first switched to golang, but after writing more, there's no need to force this anymore
Yeah, this four-layer application architecture was settled on after first asking many AIs for links to highly-starred Go web projects on GitHub, then manually reviewing them. As for the directory structure, alongside these four layers at the same level, we've also added router, response, infra, middleware, and in the future possibly dto or httpx (moving response into it) and the like