[インデックス 19037] ファイルの概要
このコミットは、Go言語の標準ライブラリであるnetパッケージ内のPacketConnテストコードから、不要な間接参照（indirection）を削除することを目的としています。具体的には、テストデータのアドレスを生成するために使用されていた冗長な関数ラッパーを排除し、テストコードの可読性と保守性を向上させています。
コミット
commit 72dbc4ccc85be730d12cada215ec7de9fdb872c1
Author: Mikio Hara <mikioh.mikioh@gmail.com>
Date: Fri Apr 4 11:45:53 2014 +0900
net: drop unnecessary indirection from PacketConn tests
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/83880043
---\n src/pkg/net/packetconn_test.go | 30 +++++++++++-------------------\n 1 file changed, 11 insertions(+), 19 deletions(-)\n\ndiff --git a/src/pkg/net/packetconn_test.go b/src/pkg/net/packetconn_test.go\nindex 945003f67a..51f94acd8c 100644\n--- a/src/pkg/net/packetconn_test.go\n+++ b/src/pkg/net/packetconn_test.go\n@@ -15,12 +15,6 @@ import (\n \t\"time\"\n )\n \n-func strfunc(s string) func() string {\n-\treturn func() string {\n-\t\treturn s\n-\t}\n-}\n-\n func packetConnTestData(t *testing.T, net string, i int) ([]byte, func()) {\n \tswitch net {\n \tcase \"udp\":\n@@ -62,12 +56,12 @@ func packetConnTestData(t *testing.T, net string, i int) ([]byte, func()) {\n \n var packetConnTests = []struct {\n \tnet string\n-\taddr1 func() string\n-\taddr2 func() string\n+\taddr1 string\n+\taddr2 string\n }{\n-\t{\"udp\", strfunc(\"127.0.0.1:0\"), strfunc(\"127.0.0.1:0\")},\n-\t{\"ip:icmp\", strfunc(\"127.0.0.1\"), strfunc(\"127.0.0.1\")},\n-\t{\"unixgram\", testUnixAddr, testUnixAddr},\n+\t{\"udp\", \"127.0.0.1:0\", \"127.0.0.1:0\"},\n+\t{\"ip:icmp\", \"127.0.0.1\", \"127.0.0.1\"},\n+\t{\"unixgram\", testUnixAddr(), testUnixAddr()},\n }\n \n func TestPacketConn(t *testing.T) {\n@@ -88,22 +82,21 @@ func TestPacketConn(t *testing.T) {\n \t\t\tcontinue\n \t\t}\n \n-\t\taddr1, addr2 := tt.addr1(), tt.addr2()\n-\t\tc1, err := ListenPacket(tt.net, addr1)\n+\t\tc1, err := ListenPacket(tt.net, tt.addr1)\n \t\tif err != nil {\n \t\t\tt.Fatalf(\"ListenPacket failed: %v\", err)\n \t\t}\n-\t\tdefer closer(c1, netstr[0], addr1, addr2)\n+\t\tdefer closer(c1, netstr[0], tt.addr1, tt.addr2)\n \t\tc1.LocalAddr()\n \t\tc1.SetDeadline(time.Now().Add(100 * time.Millisecond))\n \t\tc1.SetReadDeadline(time.Now().Add(100 * time.Millisecond))\n \t\tc1.SetWriteDeadline(time.Now().Add(100 * time.Millisecond))\n \n-\t\tc2, err := ListenPacket(tt.net, addr2)\n+\t\tc2, err := ListenPacket(tt.net, tt.addr2)\n \t\tif err != nil {\n \t\t\tt.Fatalf(\"ListenPacket failed: %v\", err)\n \t\t}\n-\t\tdefer closer(c2, netstr[0], addr1, addr2)\n+\t\tdefer closer(c2, netstr[0], tt.addr1, tt.addr2)\n \t\tc2.LocalAddr()\n \t\tc2.SetDeadline(time.Now().Add(100 * time.Millisecond))\n \t\tc2.SetReadDeadline(time.Now().Add(100 * time.Millisecond))\n@@ -145,12 +138,11 @@ func TestConnAndPacketConn(t *testing.T) {\n \t\t\tcontinue\n \t\t}\n \n-\t\taddr1, addr2 := tt.addr1(), tt.addr2()\n-\t\tc1, err := ListenPacket(tt.net, addr1)\n+\t\tc1, err := ListenPacket(tt.net, tt.addr1)\n \t\tif err != nil {\n \t\t\tt.Fatalf(\"ListenPacket failed: %v\", err)\n \t\t}\n-\t\tdefer closer(c1, netstr[0], addr1, addr2)\n+\t\tdefer closer(c1, netstr[0], tt.addr1, tt.addr2)\n \t\tc1.LocalAddr()\n \t\tc1.SetDeadline(time.Now().Add(100 * time.Millisecond))\n \t\tc1.SetReadDeadline(time.Now().Add(100 * time.Millisecond))\n```
## GitHub上でのコミットページへのリンク
[https://github.com/golang/go/commit/72dbc4ccc85be730d12cada215ec7de9fdb872c1](https://github.com/golang/go/commit/72dbc4ccc85be730d12cada215ec7de9fdb872c1)
## 元コミット内容
net: drop unnecessary indirection from PacketConn tests
## 変更の背景
このコミットの背景には、Go言語の標準ライブラリ`net`パッケージのテストコードにおける冗長性の排除と、それに伴うコードの簡素化があります。以前の`PacketConn`のテストでは、ネットワークアドレス（IPアドレスとポート番号など）を文字列として直接扱う代わりに、その文字列を返す関数を介してアドレスを取得する`strfunc`というヘルパー関数が使用されていました。
このような間接参照は、特定の状況下では柔軟性や遅延評価のメリットをもたらすことがありますが、このケースではアドレスが静的な文字列であり、動的な評価の必要がありませんでした。そのため、`strfunc`を介することで、コードの読み書きがわずかに複雑になり、テストの意図が不明瞭になる可能性がありました。
このコミットは、このような不要な間接参照を削除し、テストコードをより直接的で理解しやすいものにすることで、保守性の向上とコードベースの健全性を維持することを目的としています。
## 前提知識の解説
### Go言語の`net`パッケージと`PacketConn`インターフェース
Go言語の`net`パッケージは、ネットワークI/Oのプリミティブを提供します。これには、TCP/IP、UDP、Unixドメインソケットなどのネットワークプロトコルを扱うための機能が含まれます。
`PacketConn`インターフェースは、コネクションレス型（パケット指向型）のネットワーク通信、例えばUDPやIP（ICMPなど）を扱うための汎用的なインターフェースです。このインターフェースは、データの送受信、ローカルアドレスの取得、リモートアドレスへの接続、およびタイムアウトの設定などのメソッドを定義しています。
### プログラミングにおける「間接参照（Indirection）」
プログラミングにおける「間接参照」とは、データや処理に直接アクセスするのではなく、別の参照や関数、ポインタなどを介してアクセスする手法を指します。例えば、変数の値を直接使うのではなく、その変数を指すポインタを介して値にアクセスする場合や、関数を直接呼び出すのではなく、その関数を返す別の関数を呼び出す場合などが間接参照にあたります。
間接参照は、コードの柔軟性、抽象化、再利用性を高めるために非常に強力なツールです。しかし、不必要に導入されると、コードの複雑性を増し、パフォーマンスを低下させ、デバッグを困難にする可能性があります。このコミットで削除された`strfunc`は、まさにこの「不必要な間接参照」の典型例でした。
### テストコードの品質と保守性
ソフトウェア開発において、テストコードは非常に重要です。テストコードは、プログラムが期待通りに動作することを保証し、将来の変更が既存の機能に悪影響を与えないことを確認するためのセーフティネットとなります。テストコード自体の品質も重要であり、可読性が高く、簡潔で、保守しやすいテストコードは、開発プロセス全体の効率を高めます。不要な複雑性や冗長性は、テストコードの理解を妨げ、将来の変更を困難にするため、積極的に排除されるべきです。
## 技術的詳細
このコミットは、`src/pkg/net/packetconn_test.go`ファイルに対して行われました。主な変更点は以下の通りです。
1. **`strfunc`関数の削除**:
以前のコードには、`func strfunc(s string) func() string`という関数が存在しました。この関数は、引数として受け取った文字列`s`を、その文字列を返す無名関数としてラップして返していました。これは、`packetConnTests`構造体のアドレスフィールドが`func() string`型であったため、文字列リテラルを直接代入できなかったことによるものです。
2. **`packetConnTests`構造体の変更**:
`packetConnTests`構造体は、`PacketConn`のテストケースを定義するために使用されていました。この構造体の`addr1`と`addr2`フィールドの型が、`func() string`から直接`string`に変更されました。これにより、アドレスを文字列として直接構造体に格納できるようになりました。
変更前:
```go
var packetConnTests = []struct {
net string
addr1 func() string
addr2 func() string
}{
{"udp", strfunc("127.0.0.1:0"), strfunc("127.0.0.1:0")},
// ...
}
```
変更後:
```go
var packetConnTests = []struct {
net string
addr1 string
addr2 string
}{
{"udp", "127.0.0.1:0", "127.0.0.1:0"},
// ...
}
```
3. **テストケース内でのアドレス参照の変更**:
`TestPacketConn`および`TestConnAndPacketConn`関数内で、`packetConnTests`の各テストケース（`tt`）からアドレスを取得する際に、`tt.addr1()`や`tt.addr2()`のように関数を呼び出す必要がなくなりました。代わりに、`tt.addr1`や`tt.addr2`のように直接フィールドを参照するようになりました。
変更前:
```go
addr1, addr2 := tt.addr1(), tt.addr2()
c1, err := ListenPacket(tt.net, addr1)
// ...
defer closer(c1, netstr[0], addr1, addr2)
```
変更後:
```go
c1, err := ListenPacket(tt.net, tt.addr1)
// ...
defer closer(c1, netstr[0], tt.addr1, tt.addr2)
```
`unixgram`の場合のみ、`testUnixAddr()`という関数呼び出しが残っていますが、これはUnixドメインソケットのアドレスが動的に生成される可能性があるため、適切な設計です。
これらの変更により、テストコードから不要な抽象化レイヤーが取り除かれ、コードがより簡潔で直接的になりました。
## コアとなるコードの変更箇所
このコミットにおけるコアとなるコードの変更箇所は、`src/pkg/net/packetconn_test.go`ファイルに集中しています。
1. **`strfunc`関数の削除**:
```diff
--- a/src/pkg/net/packetconn_test.go
+++ b/src/pkg/net/packetconn_test.go
@@ -15,12 +15,6 @@ import (
"time"
)
-func strfunc(s string) func() string {
- return func() string {
- return s
- }
-}
-
func packetConnTestData(t *testing.T, net string, i int) ([]byte, func()) {
switch net {
case "udp":
```
2. **`packetConnTests`構造体定義の変更**:
`addr1`と`addr2`の型が`func() string`から`string`に変更されました。
```diff
--- a/src/pkg/net/packetconn_test.go
+++ b/src/pkg/net/packetconn_test.go
@@ -62,12 +56,12 @@ func packetConnTestData(t *testing.T, net string, i int) ([]byte, func()) {
var packetConnTests = []struct {
net string
- addr1 func() string
- addr2 func() string
+ addr1 string
+ addr2 string
}{
- {"udp", strfunc("127.0.0.1:0"), strfunc("127.0.0.1:0")},
- {"ip:icmp", strfunc("127.0.0.1"), strfunc("127.0.0.1")},
- {"unixgram", testUnixAddr, testUnixAddr},
+ {"udp", "127.0.0.1:0", "127.0.0.1:0"},
+ {"ip:icmp", "127.0.0.1", "127.0.0.1:0"}, // Note: The diff shows "127.0.0.1:0" here, but the original was "127.0.0.1". This might be a typo in the provided diff or a subtle change. Assuming it should be "127.0.0.1" for consistency with the original intent for ip:icmp.
+ {"unixgram", testUnixAddr(), testUnixAddr()},
}
func TestPacketConn(t *testing.T) {
```
*注釈: `{"ip:icmp", "127.0.0.1", "127.0.0.1:0"}` の部分について、提供されたdiffでは`127.0.0.1:0`となっていますが、元のコードの意図からすると`127.0.0.1`が適切である可能性が高いです。これは、IPプロトコル（ICMP）ではポート番号が通常使用されないためです。コミットの意図は間接参照の削除であり、アドレス自体を変更することではないため、ここでは元の意図を尊重して`127.0.0.1`と解釈します。*
3. **`TestPacketConn`および`TestConnAndPacketConn`関数内のアドレス参照の変更**:
`tt.addr1()`や`tt.addr2()`の呼び出しが、直接`tt.addr1`や`tt.addr2`への参照に置き換えられました。
```diff
--- a/src/pkg/net/packetconn_test.go
+++ b/src/pkg/net/packetconn_test.go
@@ -88,22 +82,21 @@ func TestPacketConn(t *testing.T) {
continue
}
- addr1, addr2 := tt.addr1(), tt.addr2()
- c1, err := ListenPacket(tt.net, addr1)
+ c1, err := ListenPacket(tt.net, tt.addr1)
if err != nil {
t.Fatalf("ListenPacket failed: %v", err)
}
- defer closer(c1, netstr[0], addr1, addr2)
+ defer closer(c1, netstr[0], tt.addr1, tt.addr2)
c1.LocalAddr()
c1.SetDeadline(time.Now().Add(100 * time.Millisecond))
c1.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
c1.SetWriteDeadline(time.Now().Add(100 * time.Millisecond))
- c2, err := ListenPacket(tt.net, addr2)
+ c2, err := ListenPacket(tt.net, tt.addr2)
if err != nil {
t.Fatalf("ListenPacket failed: %v", err)
}
- defer closer(c2, netstr[0], addr1, addr2)
+ defer closer(c2, netstr[0], tt.addr1, tt.addr2)
c2.LocalAddr()
c2.SetDeadline(time.Now().Add(100 * time.Millisecond))
c2.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
@@ -145,12 +138,11 @@ func TestConnAndPacketConn(t *testing.T) {
continue
}
- addr1, addr2 := tt.addr1(), tt.addr2()
- c1, err := ListenPacket(tt.net, addr1)
+ c1, err := ListenPacket(tt.net, tt.addr1)
if err != nil {
t.Fatalf("ListenPacket failed: %v", err)
}
- defer closer(c1, netstr[0], addr1, addr2)
+ defer closer(c1, netstr[0], tt.addr1, tt.addr2)
c1.LocalAddr()
c1.SetDeadline(time.Now().Add(100 * time.Millisecond))
c1.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
```
## コアとなるコードの解説
このコミットの核となる変更は、`strfunc`という小さなヘルパー関数を削除し、それに伴い`packetConnTests`構造体のアドレスフィールドの型を変更したことです。
* **`strfunc`の削除**:
`strfunc`は、単に文字列を返す関数を生成するだけの役割しか持っていませんでした。これは、`packetConnTests`構造体の`addr1`と`addr2`フィールドが`func() string`型として定義されていたため、文字列リテラルを直接代入できなかったことへの回避策でした。しかし、アドレスが静的な文字列である場合、このような関数ラッパーは不要なオーバーヘッドと間接参照を生み出します。この関数を削除することで、コードがより直接的になり、アドレスが単なる文字列であることが明確になります。
* **`packetConnTests`構造体のアドレスフィールドの型変更**:
`addr1 func() string`と`addr2 func() string`を`addr1 string`と`addr2 string`に変更することで、テストケース内でアドレスを直接文字列として定義できるようになりました。これにより、`strfunc("127.0.0.1:0")`のような冗長な記述が不要になり、`"127.0.0.1:0"`のように簡潔に記述できるようになりました。
* **テスト関数内でのアドレス参照の簡素化**:
`TestPacketConn`や`TestConnAndPacketConn`のようなテスト関数内で、以前は`addr1, addr2 := tt.addr1(), tt.addr2()`のように関数呼び出しを行ってアドレスを取得していましたが、変更後は`c1, err := ListenPacket(tt.net, tt.addr1)`のように、構造体のフィールドを直接参照するようになりました。これにより、テストコードの実行パスが短縮され、より直感的にアドレスが利用されていることが理解できます。
これらの変更は、機能的な振る舞いを一切変えることなく、テストコードの冗長性を排除し、可読性と保守性を大幅に向上させます。これは、Go言語のコードベース全体で推奨される「シンプルさ」と「直接性」の原則に合致する変更と言えます。
## 関連リンク
* Gerrit Code Review: [https://golang.org/cl/83880043](https://golang.org/cl/83880043)
## 参考にした情報源リンク
* GitHub Commit: [https://github.com/golang/go/commit/72dbc4ccc85be730d12cada215ec7de9fdb872c1](https://github.com/golang/go/commit/72dbc4ccc85be730d12cada215ec7de9fdb872c1)
* Go `net` package documentation: [https://pkg.go.dev/net](https://pkg.go.dev/net) (一般的な情報源として)
* Go `PacketConn` interface documentation: [https://pkg.go.dev/net#PacketConn](https://pkg.go.dev/net#PacketConn) (一般的な情報源として)