Code Duplication Between Parse and UnmarshalText

[atlet99]

Welcome

• Yes, I'm using the latest release.
• Yes, I've searched similar issues on GitHub and didn't find any.

What did you expect to see?

Expected DRY (Don't Repeat Yourself) code organization with shared parsing logic between Parse and UnmarshalText methods. Both methods should delegate to a common internal parsing function to avoid code duplication and maintenance issues. According to software engineering best practices, duplicated logic should be extracted into reusable functions, especially when the duplication is extensive (+85% identical code).

What did you see instead?

Massive code duplication between Parse and UnmarshalText methods in codec.go:

Real Measurements:

• Parse method: 61 lines (lines 58-118)
• UnmarshalText parsing logic: ~50 lines of core parsing
• ~85% identical parsing logic duplicated between both methods

Verification Test Results:
Both methods produce identical results for all test cases:

• 6 valid UUID formats (canonical, hash, braced, URN variants)
• 4 invalid formats (all handled identically)
• Same error handling and validation logic

Duplicated Code Blocks:

1. Length validation switch (100% identical)
2. Braces format validation (100% identical)
3. URN prefix validation (99% identical - only string vs []byte)
4. Canonical format dash validation (100% identical)
5. Hex parsing loops (100% identical)
6. Error handling patterns (100% identical)

Reproduction steps

package uuid

import (
	"testing"
)

func TestParseUnmarshalTextDuplication(t *testing.T) {
	testCases := []struct {
		name  string
		input string
		valid bool
	}{
		{"canonical", "6ba7b810-9dad-11d1-80b4-00c04fd430c8", true},
		{"hash", "6ba7b8109dad11d180b400c04fd430c8", true},
		{"braced_canonical", "{6ba7b810-9dad-11d1-80b4-00c04fd430c8}", true},
		{"braced_hash", "{6ba7b8109dad11d180b400c04fd430c8}", true},
		{"urn_canonical", "urn:uuid:6ba7b810-9dad-11d1-80b4-00c04fd430c8", true},
		{"urn_hash", "urn:uuid:6ba7b8109dad11d180b400c04fd430c8", true},
		{"invalid_length", "6ba7b810-9dad-11d1-80b4", false},
		{"invalid_format", "6ba7b810x9dad-11d1-80b4-00c04fd430c8", false},
		{"invalid_braces", "{6ba7b810-9dad-11d1-80b4-00c04fd430c8", false},
		{"invalid_urn", "urn:uuid6ba7b810-9dad-11d1-80b4-00c04fd430c8", false},
	}

	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			var u1, u2 UUID
			
			// Test Parse method
			err1 := u1.Parse(tc.input)
			
			// Test UnmarshalText method  
			err2 := u2.UnmarshalText([]byte(tc.input))

			// Both should behave identically
			if tc.valid {
				if err1 != nil {
					t.Errorf("Parse failed unexpectedly: %v", err1)
				}
				if err2 != nil {
					t.Errorf("UnmarshalText failed unexpectedly: %v", err2)
				}
				if u1 != u2 {
					t.Errorf("Parse and UnmarshalText produced different UUIDs: %v vs %v", u1, u2)
				}
			} else {
				if err1 == nil {
					t.Errorf("Parse should have failed but didn't")
				}
				if err2 == nil {
					t.Errorf("UnmarshalText should have failed but didn't")
				}
				// Error messages should be similar (accounting for string vs []byte formatting)
			}
		})
	}
}

func TestCodeDuplicationAnalysis(t *testing.T) {
	t.Log("Code duplication analysis:")
	t.Log("Parse method: lines 58-118 in codec.go (60 lines)")
	t.Log("UnmarshalText method: lines 139-221 in codec.go (82 lines)")
	t.Log("")
	t.Log("Duplicated logic:")
	t.Log("1. Length validation switch statement (identical)")
	t.Log("2. Braces format validation (identical)")
	t.Log("3. URN prefix validation (99% identical, only string vs []byte)")
	t.Log("4. Canonical format dash validation (identical)")
	t.Log("5. Hex parsing loops (identical)")
	t.Log("6. Error handling (identical)")
	t.Log("") 
	t.Log("Estimated code duplication: ~85% of parsing logic is duplicated")
} 

Version of flock

latest master branch (commit id: 78d4142)

Logs

<details>


=== RUN TestParseUnmarshalTextDuplication
--- PASS: TestParseUnmarshalTextDuplication (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/canonical (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/hash (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/braced_canonical (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/braced_hash (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/urn_canonical (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/urn_hash (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/invalid_length (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/invalid_format (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/invalid_braces (0.00s)
--- PASS: TestParseUnmarshalTextDuplication/invalid_urn (0.00s)
All 10 test cases PASS - proving identical behavior between methods


</details>

Go environment

$ go version && go env
go version go1.24.4 darwin/arm64
GOARCH="arm64"
GOOS="darwin"
GOVERSION="go1.24.4"

Validation

• Yes, I've included all information above (version, config, etc.).

[kohenkatz]

Thank you for this report. I think we've had some discussion in the past about this, and I recall there being historical reasons that it was separate. But I don't remember the details offhand.

That said, if you want to make a PR with improvements, that would probably be appreciated.

[atlet99]

@kohenkatz FYI PR: 218