cv-site/_go-learning/refactorings/001-cv-model-separation.md

# Refactoring #001: CV Model and UI Model Separation

**Date**: 2025-11-20
**Status**: In Progress
**Complexity**: Medium
**Learning Value**: ⭐⭐⭐⭐⭐

## 📋 Table of Contents
1. [The Problem](#the-problem)
2. [Why This Matters](#why-this-matters)
3. [The Solution](#the-solution)
4. [Deep Dive: Go Package Philosophy](#deep-dive-go-package-philosophy)
5. [Architecture Diagrams](#architecture-diagrams)
6. [Implementation Steps](#implementation-steps)
7. [Testing Strategy](#testing-strategy)
8. [Lessons Learned](#lessons-learned)

---

## 🔴 The Problem

### Current State

The file `internal/models/cv.go` (301 lines) contains **two completely different concerns**:

1. **Domain Models (lines 13-158)**: Business logic and CV data structures
   - `CV`, `Personal`, `Experience`, `Education`, `Skills`, `Language`, `Project`, etc.
   - Represents the **core business domain** of a curriculum vitae
   - Data that comes from `data/cv-{lang}.json`

2. **Presentation Models (lines 160-215)**: UI configuration and translations
   - `UI`, `InfoModal`, `ShortcutsModal`, `TechStack`, `ShortcutGroup`, etc.
   - Represents **user interface state** and internationalization
   - Data that comes from `data/ui-{lang}.json`

### Why Is This a Problem?

```go
// Current: Everything mixed together
package models

type CV struct { ... }           // Business domain
type Experience struct { ... }   // Business domain
type UI struct { ... }           // Presentation layer!?
type InfoModal struct { ... }    // Presentation layer!?
```

**Violations**:
- ❌ **Single Responsibility Principle**: One file doing two jobs
- ❌ **Separation of Concerns**: Business logic mixed with UI logic
- ❌ **Scalability**: Hard to grow either domain independently
- ❌ **Testability**: Can't test CV logic without UI types in scope
- ❌ **Clarity**: New developers confused about boundaries

---

## 🎯 Why This Matters

### 1. **Separation of Concerns** (Fundamental Design Principle)

> "A module should be responsible to one, and only one, actor." - Robert C. Martin (Uncle Bob)

In our case, we have **two actors**:
- **Business stakeholders**: Care about CV data structure, validation, completeness
- **UI/UX designers**: Care about modal content, keyboard shortcuts, translations

Mixing these concerns means:
- Changes to UI translations force recompilation of CV business logic
- Testing CV data loading requires UI types in memory
- Can't reason about one domain without understanding the other

### 2. **Go's Package Philosophy**

Go encourages **small, focused packages** that do one thing well:

```go
// Go standard library examples
import "net/http"        // HTTP client/server
import "encoding/json"   // JSON encoding
import "html/template"   // HTML templating

// NOT like this (anti-pattern):
import "net/everything"  // HTTP, WebSocket, RPC, all mixed
```

**Key Insight**: Go packages are the **primary means of abstraction**. Unlike Java/C# where classes are primary, in Go you think in packages.

### 3. **Dependency Management**

```
Current (Bad):
┌─────────────────────────────────┐
│    internal/handlers/cv.go      │
│  (imports "models" - gets BOTH  │
│   CV domain AND UI presentation)│
└─────────────────────────────────┘
              ▼
┌─────────────────────────────────┐
│    internal/models/cv.go        │
│  CV domain + UI presentation    │
│  (300+ lines, mixed concerns)   │
└─────────────────────────────────┘

Future (Good):
┌─────────────────────────────────┐
│    internal/handlers/cv.go      │
│  (imports BOTH packages, but    │
│   can choose what to import)    │
└─────────────────────────────────┘
           ▼           ▼
    ┌──────────┐  ┌──────────┐
    │ models/cv│  │ models/ui│
    │  (domain)│  │ (present)│
    └──────────┘  └──────────┘
```

**Benefits**:
- Handlers can import just `models/cv` if they don't need UI
- PDF generator doesn't need to know about modals
- API endpoints can return CV data without UI overhead

### 4. **Testing Independence**

```go
// With separated packages, you can test CV logic without UI:

// cv/loader_test.go
func TestLoadCV(t *testing.T) {
    cv, err := cv.LoadCV("en")
    // Test ONLY CV business logic
    // No UI types needed, faster compilation
}

// ui/loader_test.go
func TestLoadUI(t *testing.T) {
    ui, err := ui.LoadUI("en")
    // Test ONLY UI translations
    // No CV types needed
}
```

### 5. **Scalability**

As the project grows:

```
CV domain might add:
- Validation logic
- Export formats (PDF, Word, LinkedIn)
- Version control
- Analytics
- Recommendations engine

UI domain might add:
- More modals
- Theme configurations
- Accessibility settings
- User preferences
- A/B testing configs

With separation: Each grows independently
Without separation: 1000+ line monolithic file
```

---

## 💡 The Solution

### Target Package Structure

```
internal/models/
├── cv/                          # CV Domain Package
│   ├── cv.go                   # Core CV types (CV, Personal, etc.)
│   ├── loader.go               # LoadCV() + data loading logic
│   └── loader_test.go          # Unit tests for CV loading
│
├── ui/                          # UI Presentation Package
│   ├── ui.go                   # UI types (UI, InfoModal, etc.)
│   ├── loader.go               # LoadUI() + data loading logic
│   └── loader_test.go          # Unit tests for UI loading
│
└── cv.go (DEPRECATED)          # Optional compatibility layer
```

### Why This Structure?

#### 1. **Package Names Reveal Intent**
```go
import "github.com/juanatsap/cv-site/internal/models/cv"
import "github.com/juanatsap/cv-site/internal/models/ui"
```

Just from the import, you know:
- `cv` = Business domain logic
- `ui` = Presentation layer logic

#### 2. **File Names Are Self-Documenting**
```
cv/loader.go      → "This loads CV data"
ui/loader.go      → "This loads UI translations"
```

No need to read 300 lines to find what you need.

#### 3. **Tests Live Alongside Code** (Go Convention)
```
cv/cv.go          → CV type definitions
cv/loader.go      → CV loading logic
cv/loader_test.go → Tests for loader.go
```

Go's tooling expects `*_test.go` files in the same directory as the code they test.

---

## 🏗️ Deep Dive: Go Package Philosophy

### What Makes a Good Go Package?

#### Principle 1: **Cohesion**
> "Things that change together should be packaged together."

**Good**: All CV types in one package (they change when business requirements change)
**Bad**: CV types mixed with UI types (they change for different reasons)

#### Principle 2: **Minimal API Surface**
> "Export only what's necessary."

```go
// cv/loader.go

// Exported (public API)
func LoadCV(lang string) (*CV, error) { ... }

// Unexported (internal helper)
func findDataFile(filename string) (string, error) { ... }
func replaceYearPlaceholder(url, year string) string { ... }
```

**Why unexport helpers?**
- Smaller public API = easier to maintain
- Can refactor internals without breaking clients
- Forces callers to use the high-level `LoadCV()` interface

#### Principle 3: **No Circular Dependencies**
Go compiler **forbids** package cycles:

```go
// This will NOT compile:
package cv
import "ui"  // cv depends on ui

package ui
import "cv"  // ui depends on cv
// ERROR: import cycle not allowed
```

**Our design avoids this**:
```
handlers → cv
handlers → ui
cv → (nothing)
ui → (nothing)
```

Clean one-way dependency flow!

#### Principle 4: **Package Names Are Part of the API**

```go
// BAD: Stutter
cv.CVLoader()       // "CV" mentioned twice
cv.LoadCVData()     // Redundant

// GOOD: Clear, concise
cv.LoadCV()         // Package name + function name = clear intent
ui.LoadUI()         // Same pattern
```

When you import `cv`, it's obvious everything in it relates to CV domain.

---

## 📊 Architecture Diagrams

### Before Refactoring (Current State)

```
┌─────────────────────────────────────────────────────────────┐
│                     main.go                                 │
│                  (Server startup)                           │
└─────────────────────────────────────────────────────────────┘
                           │
                           ▼
┌─────────────────────────────────────────────────────────────┐
│              internal/handlers/cv.go                        │
│         (HTTP handlers for CV endpoints)                    │
│                                                             │
│  import "github.com/.../internal/models"                   │
│                                                             │
│  cv, _ := models.LoadCV("en")   ← Gets CV data            │
│  ui, _ := models.LoadUI("en")   ← Gets UI translations    │
└─────────────────────────────────────────────────────────────┘
                           │
                           ▼
┌─────────────────────────────────────────────────────────────┐
│              internal/models/cv.go                          │
│                   (300+ LINES)                              │
│  ┌────────────────────────────────────────────────────┐   │
│  │  CV Domain Models (lines 13-158)                   │   │
│  │  - type CV struct { ... }                          │   │
│  │  - type Personal struct { ... }                    │   │
│  │  - type Experience struct { ... }                  │   │
│  │  - func LoadCV(lang) (*CV, error)                  │   │
│  └────────────────────────────────────────────────────┘   │
│                                                             │
│  ┌────────────────────────────────────────────────────┐   │
│  │  UI Presentation Models (lines 160-215)            │   │
│  │  - type UI struct { ... }                          │   │
│  │  - type InfoModal struct { ... }                   │   │
│  │  - type ShortcutsModal struct { ... }              │   │
│  │  - func LoadUI(lang) (*UI, error)                  │   │
│  └────────────────────────────────────────────────────┘   │
│                                                             │
│  PROBLEM: Two concerns mixed in one file!                  │
└─────────────────────────────────────────────────────────────┘
                    │                    │
                    ▼                    ▼
            ┌──────────────┐    ┌──────────────┐
            │ data/cv-*.json│    │data/ui-*.json│
            │ (CV data)     │    │ (UI text)    │
            └──────────────┘    └──────────────┘
```

**Issues**:
- ❌ Single file with multiple responsibilities
- ❌ Can't import CV without also importing UI types
- ❌ Hard to test CV logic in isolation
- ❌ Confusing for new developers

### After Refactoring (Target State)

```
┌─────────────────────────────────────────────────────────────┐
│                     main.go                                 │
│                  (Server startup)                           │
└─────────────────────────────────────────────────────────────┘
                           │
                           ▼
┌─────────────────────────────────────────────────────────────┐
│              internal/handlers/cv.go                        │
│         (HTTP handlers for CV endpoints)                    │
│                                                             │
│  import cvmodel "github.com/.../internal/models/cv"        │
│  import uimodel "github.com/.../internal/models/ui"        │
│                                                             │
│  cv, _ := cvmodel.LoadCV("en")   ← Clear: CV domain       │
│  ui, _ := uimodel.LoadUI("en")   ← Clear: UI presentation │
└─────────────────────────────────────────────────────────────┘
                    │                    │
         ┌──────────┴────────┐           │
         ▼                   ▼           ▼
┌─────────────────┐  ┌─────────────────────────┐
│ internal/models/│  │ internal/models/ui/     │
│ cv/             │  │                         │
│                 │  │  ┌───────────────────┐  │
│ ┌─────────────┐ │  │  │ ui.go             │  │
│ │ cv.go       │ │  │  │ - type UI         │  │
│ │ - type CV   │ │  │  │ - type InfoModal  │  │
│ │ - Personal  │ │  │  │ - type Shortcuts  │  │
│ │ - Experience│ │  │  └───────────────────┘  │
│ └─────────────┘ │  │                         │
│                 │  │  ┌───────────────────┐  │
│ ┌─────────────┐ │  │  │ loader.go         │  │
│ │ loader.go   │ │  │  │ - LoadUI(lang)    │  │
│ │ - LoadCV()  │ │  │  │ - findDataFile()  │  │
│ └─────────────┘ │  │  └───────────────────┘  │
│                 │  │                         │
│ ┌─────────────┐ │  │  ┌───────────────────┐  │
│ │loader_test  │ │  │  │ loader_test.go    │  │
│ │ - TestLoadCV│ │  │  │ - TestLoadUI      │  │
│ └─────────────┘ │  │  └───────────────────┘  │
└─────────────────┘  └─────────────────────────┘
         │                        │
         ▼                        ▼
┌──────────────┐          ┌──────────────┐
│data/cv-*.json│          │data/ui-*.json│
│ (CV data)    │          │ (UI text)    │
└──────────────┘          └──────────────┘
```

**Benefits**:
- ✅ Clear separation of concerns
- ✅ Can import `cv` without `ui` (and vice versa)
- ✅ Independent testing of each domain
- ✅ Easier to navigate and understand
- ✅ Scales as project grows

### Dependency Graph

```
                    Templates
                    (*.html)
                       ▲
                       │ (runtime reflection,
                       │  no compile-time deps)
                       │
              ┌────────┴────────┐
              │                 │
      internal/handlers/cv.go   │
              │                 │
              │                 │
    ┌─────────┴─────────┐       │
    │                   │       │
    ▼                   ▼       │
┌─────────┐      ┌──────────┐  │
│models/cv│      │models/ui │  │
│         │      │          │  │
│ (domain)│      │(present) │  │
└─────────┘      └──────────┘  │
    │                   │       │
    └─────────┬─────────┘       │
              ▼                 │
        JSON Data Files         │
        ┌──────────────┐        │
        │ cv-*.json    │        │
        │ ui-*.json    │        │
        └──────────────┘        │
                                │
                                │
              Static Assets ────┘
              (CSS, images)
```

**Key Observations**:
- No circular dependencies
- `cv` and `ui` packages are independent (parallel)
- Handlers orchestrate both domains
- Templates have no compile-time dependencies

---

## 🔧 Implementation Steps

### Phase 1: Create New Package Structure ✅

```bash
mkdir -p internal/models/cv
mkdir -p internal/models/ui
```

### Phase 2: Extract CV Domain Types ✅

**File: `internal/models/cv/cv.go`**
- Move: `CV`, `Personal`, `Experience`, `Education`, `Skills`, `SkillCategory`, `Language`, `Project`, `Award`, `Certification`, `Course`, `Reference`, `Other`, `Meta`
- Keep: JSON tags, struct tags, comments

**File: `internal/models/cv/loader.go`**
- Move: `LoadCV()` function
- Move: `findDataFile()` helper
- Move: `replaceYearPlaceholder()` helper
- Add: Package-level documentation

### Phase 3: Extract UI Presentation Types ✅

**File: `internal/models/ui/ui.go`**
- Move: `UI`, `InfoModal`, `TechStack`, `ShortcutsModal`, `ShortcutsSections`, `ShortcutGroup`, `ShortcutItem`

**File: `internal/models/ui/loader.go`**
- Move: `LoadUI()` function
- Duplicate: `findDataFile()` helper (or create shared util)

### Phase 4: Update Handler Imports 🔄

**File: `internal/handlers/cv.go`**

```go
// Before:
import "github.com/juanatsap/cv-site/internal/models"

// After:
import (
    cvmodel "github.com/juanatsap/cv-site/internal/models/cv"
    uimodel "github.com/juanatsap/cv-site/internal/models/ui"
)
```

**Why the aliases?**
- `cvmodel` and `uimodel` prevent shadowing of `cv` and `ui` variables
- Common Go pattern when package name conflicts with variable names

### Phase 5: Create Tests ✅

**File: `internal/models/cv/loader_test.go`**
- Test `LoadCV()` for both languages
- Test error cases (invalid language, missing file)
- Test year placeholder replacement

**File: `internal/models/ui/loader_test.go`**
- Test `LoadUI()` for both languages
- Test error cases

### Phase 6: Validation 🔄

```bash
# Compile check
go build ./...

# Run tests
go test ./internal/models/cv/...
go test ./internal/models/ui/...

# Start server
make run

# Test endpoints
curl http://localhost:1999/?lang=en
curl http://localhost:1999/?lang=es
```

---

## 🧪 Testing Strategy

### Test Organization

```
Frontend Tests (EXISTING - DON'T TOUCH):
tests/
├── mjs/
│   ├── 01-*.test.mjs       # E2E tests
│   ├── 29-pdf-toast-*.test.mjs
│   └── ...
└── (Bun/JavaScript tests for full UX)

Backend Tests (NEW - GO TESTS):
internal/
├── models/
│   ├── cv/
│   │   ├── cv.go
│   │   ├── loader.go
│   │   └── loader_test.go    # Unit tests for CV package
│   └── ui/
│       ├── ui.go
│       ├── loader.go
│       └── loader_test.go    # Unit tests for UI package
└── handlers/
    ├── cv.go
    └── cv_test.go             # Integration tests for handlers
```

### Test Types

#### 1. **Unit Tests** (Fast, Isolated)
```go
// internal/models/cv/loader_test.go
func TestLoadCV_ValidLanguage(t *testing.T) {
    cv, err := LoadCV("en")
    if err != nil {
        t.Fatalf("LoadCV failed: %v", err)
    }
    if cv.Personal.Name == "" {
        t.Error("Expected CV to have a name")
    }
}
```

**Purpose**: Test individual functions in isolation

#### 2. **Integration Tests** (Medium Speed)
```go
// internal/handlers/cv_test.go
func TestHomeHandler_ReturnsCV(t *testing.T) {
    req := httptest.NewRequest("GET", "/?lang=en", nil)
    w := httptest.NewRecorder()

    handler.Home(w, req)

    if w.Code != 200 {
        t.Errorf("Expected 200, got %d", w.Code)
    }
}
```

**Purpose**: Test how components work together

#### 3. **E2E Tests** (Existing, Don't Touch)
```javascript
// tests/mjs/01-*.test.mjs
test('CV loads in English', async () => {
    await page.goto('http://localhost:1999/?lang=en');
    await expect(page.locator('h1')).toContainText('CV');
});
```

**Purpose**: Test full user experience (UI + backend + interactions)

### Why This Separation?

| Test Type | Speed | Scope | When to Run |
|-----------|-------|-------|-------------|
| **Go Unit** | ⚡ Fast (ms) | Single function | Every save, pre-commit |
| **Go Integration** | 🏃 Medium (100ms) | Multiple components | Pre-push, CI |
| **E2E Frontend** | 🐌 Slow (seconds) | Full application | Pre-deploy, nightly |

**Philosophy**:
- **Unit tests**: Catch bugs early, run constantly
- **Integration tests**: Verify components work together
- **E2E tests**: Ensure user experience is intact

---

## 📚 Lessons Learned

### 1. **Go Package Design Is an Art**

Creating packages isn't about file organization—it's about **domain boundaries**.

**Bad approach**: "Let's split cv.go because it's too long"
**Good approach**: "Let's separate CV domain from UI domain because they have different responsibilities"

### 2. **Duplication vs. Abstraction**

We duplicated `findDataFile()` in both `cv/loader.go` and `ui/loader.go`.

**Why?**
- It's a small function (17 lines)
- Creates **package independence** (no shared dependencies)
- Avoids premature abstraction

**Rule of thumb**: "Duplication is far cheaper than the wrong abstraction." - Sandi Metz

### 3. **Import Aliases Save Pain**

```go
// Without aliases (BAD):
import "github.com/.../models/cv"

func Home() {
    cv, _ := cv.LoadCV("en")  // ERROR: cv.cv? Confusing!
}

// With aliases (GOOD):
import cvmodel "github.com/.../models/cv"

func Home() {
    cv, _ := cvmodel.LoadCV("en")  // Clear!
}
```

### 4. **Tests Are Part of the Public API**

In Go, tests live alongside the code. This forces you to think about:
- What's exported (public)?
- What's unexported (private)?
- How will clients use this package?

### 5. **Refactoring Is Iterative**

We could further refactor:
- Split `cv/cv.go` into multiple files (`personal.go`, `experience.go`, etc.)
- Extract validation logic
- Add builder patterns

But we stopped here because:
- ✅ Addresses the immediate problem (mixed concerns)
- ✅ Provides clear boundaries
- ✅ Leaves room for future improvements
- ✅ Doesn't over-engineer

---

## 🎓 Key Takeaways for Job Interviews

### When Asked About Go Package Design:

**Question**: "How do you organize Go code?"

**Answer Framework**:
1. **Start with the domain**: What are the core concepts? (CV, UI, etc.)
2. **Identify responsibilities**: What changes together? What changes for different reasons?
3. **Create boundaries**: Packages represent domain boundaries, not file organization
4. **Follow Go idioms**: Small, focused packages with clear names
5. **Avoid circular dependencies**: Design for one-way dependency flow

### Example Response:

> "In my CV project, I refactored a monolithic `models/cv.go` file that mixed business domain (CV data) with presentation logic (UI translations). I split it into two packages: `models/cv` for the business domain and `models/ui` for presentation.
>
> This followed Go's philosophy of small, focused packages and made the code more testable—I could now test CV logic without importing UI types. The key insight was recognizing these were two different domains with different change drivers: CV structure changes when business requirements change, while UI changes when designers update the interface.
>
> I also ensured no circular dependencies by having both packages be leaf nodes in the dependency graph, with handlers orchestrating both."

### When Asked About Refactoring:

**Question**: "Tell me about a significant refactoring you did."

**Answer Framework**:
1. **Identify the problem**: What was wrong? Why did it matter?
2. **Design the solution**: What principles guided your approach?
3. **Execute incrementally**: How did you minimize risk?
4. **Validate the change**: How did you ensure it worked?
5. **Measure the impact**: What improved?

**Metrics for this refactoring**:
- Lines per file: 300+ → ~100 (more manageable)
- Test isolation: Impossible → Easy (independent domains)
- Compilation time: Unchanged (actually slightly faster with parallel compilation)
- Maintainability: Improved (clear boundaries)

---

## 📈 Next Steps

### Potential Future Improvements

1. **Further file splitting**:
   ```
   cv/
   ├── cv.go          # Core CV type
   ├── personal.go    # Personal info types
   ├── experience.go  # Work experience types
   ├── skills.go      # Skills types
   └── loader.go      # Data loading
   ```

2. **Add validation**:
   ```go
   // cv/validation.go
   func (cv *CV) Validate() error {
       if cv.Personal.Name == "" {
           return errors.New("name is required")
       }
       // ...
   }
   ```

3. **Introduce interfaces**:
   ```go
   type CVRepository interface {
       LoadCV(lang string) (*CV, error)
       SaveCV(cv *CV, lang string) error
   }
   ```

4. **Add builders**:
   ```go
   cv := cv.NewBuilder().
       WithPersonal(personal).
       WithExperience(experiences).
       Build()
   ```

But remember: **Don't over-engineer**. Add complexity only when you need it.

---

## 📖 Further Reading

### Go Package Design
- [Go Blog: Package names](https://go.dev/blog/package-names)
- [Effective Go: Packages](https://go.dev/doc/effective_go#packages)
- [Go Proverbs: Clear is better than clever](https://go-proverbs.github.io/)

### Software Design Principles
- **Single Responsibility Principle** (SRP)
- **Separation of Concerns** (SoC)
- **Dependency Inversion Principle** (DIP)

### Books
- "The Go Programming Language" - Donovan & Kernighan (Chapter 10: Packages)
- "Clean Architecture" - Robert C. Martin (Principles apply to Go)

---

**Last Updated**: 2025-11-20
**Status**: Implementing
**Confidence**: High (well-established patterns)