docs: add when/when-not to data-transforms

2026-04-30 05:38:33 -07:00
parent f23623250e
commit cb94a157a1
7 changed files with 3278 additions and 414 deletions
@@ -0,0 +1,425 @@
+#!/usr/bin/env python3
+"""Add When to Use / When NOT to Use sections to modules.md"""
+
+with open("patterns/modules.md", "r") as f:
+    content = f.read()
+
+separator = "\n\n---\n\n"
+parts = content.split(separator)
+print(f"Found {len(parts)} parts")
+
+when_sections = {
+    1: '''
+### When to Use
+
+**Triggers:**
+- Your module has grown beyond ~300 lines with distinct sub-responsibilities
+- External code only needs the parent module but implementation is complex
+- You find yourself prefixing private functions with a concept name (e.g., `scope_push`, `scope_pop`)
+
+**Example — before:**
+```elixir
+# Everything crammed into one flat module
+defmodule MyApp.Router do
+  # 800 lines mixing route compilation, scope tracking, and helper generation
+  def compile_route(...), do: # ...
+  def push_scope(...), do: # ...
+  def pop_scope(...), do: # ...
+  def generate_helper(...), do: # ...
+end
+```
+
+**Example — after:**
+```elixir
+# Parent module is the public API
+defmodule MyApp.Router do
+  # Public API delegates to focused submodules
+  def compile(routes), do: MyApp.Router.Compiler.compile(routes)
+end
+
+# Submodules handle implementation
+defmodule MyApp.Router.Compiler do
+  @moduledoc false
+  # ...
+end
+
+defmodule MyApp.Router.Scope do
+  @moduledoc false
+  # ...
+end
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- The module is small and cohesive (< 200 lines)
+- Nesting would exceed 3 levels (`A.B.C.D` is usually too deep)
+- The "submodule" has its own independent public API (make it a sibling instead)
+
+**Over-application example:**
+```elixir
+# Over-nesting a simple utility
+defmodule MyApp.Utils.String.Formatting.Case do
+  def upcase(s), do: String.upcase(s)
+end
+```
+
+**Better alternative:**
+```elixir
+defmodule MyApp.StringUtils do
+  def upcase(s), do: String.upcase(s)
+end
+```
+
+**Why:** Nesting should reflect genuine conceptual hierarchy. If you're creating submodules for 2-3 functions that don't have independent complexity, you're adding navigational overhead without architectural benefit.''',
+
+    2: '''
+### When to Use
+
+**Triggers:**
+- You're writing a new module and need to decide function ordering
+- A module has grown organically and functions are scattered randomly
+- You're reviewing code and finding it hard to locate the public API
+
+**Example — before:**
+```elixir
+defmodule UserService do
+  defp hash_password(pw), do: # ...
+
+  def create(attrs) do
+    # uses hash_password
+  end
+
+  def start_link(opts), do: GenServer.start_link(__MODULE__, opts)
+
+  defp validate(attrs), do: # ...
+
+  def get(id), do: # ...
+end
+```
+
+**Example — after:**
+```elixir
+defmodule UserService do
+  # Lifecycle
+  def start_link(opts), do: GenServer.start_link(__MODULE__, opts)
+
+  # Public API
+  def create(attrs), do: # ...
+  def get(id), do: # ...
+
+  # Private helpers
+  defp validate(attrs), do: # ...
+  defp hash_password(pw), do: # ...
+end
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- You have a tiny module (< 5 functions) where ordering doesn't matter much
+- The module is a pure data module (just a struct + typespec)
+- "Logical grouping" puts closely related public+private pairs together for readability
+
+**Over-application example:**
+```elixir
+# Forcing start_link to the top in a module that isn't an OTP process
+defmodule MyApp.Parser do
+  # This module has no lifecycle — don't force OTP ordering
+  def start_link(_), do: raise "not a process"  # Just to match the pattern?
+  def parse(input), do: # ...
+end
+```
+
+**Better alternative:**
+```elixir
+defmodule MyApp.Parser do
+  @moduledoc "Parses input format X into structs"
+
+  def parse(input), do: # ...
+  def parse!(input), do: # ...
+
+  defp tokenize(input), do: # ...
+end
+```
+
+**Why:** The ordering convention exists to make OTP-aware modules predictable. For non-OTP modules, lead with the primary public function (the one callers reach for first) and let the rest follow logically.''',
+
+    3: '''
+### When to Use
+
+**Triggers:**
+- A module exists purely for internal code organization
+- Users of your library should never call this module directly
+- The module is a helper that could change or disappear between versions
+
+**Example — before:**
+```elixir
+defmodule MyApp.Repo.QueryBuilder do
+  @moduledoc """
+  Builds Ecto queries for the Repo module.
+  """
+  # Now appears in docs, users try to call it directly
+end
+```
+
+**Example — after:**
+```elixir
+defmodule MyApp.Repo.QueryBuilder do
+  @moduledoc false
+  # Hidden from docs, clearly internal
+end
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- The module is part of your public API (even if rarely used)
+- Users need to implement callbacks or extend the module
+- The module defines a behaviour or protocol that others implement
+
+**Over-application example:**
+```elixir
+# Hiding a module that users actually need
+defmodule MyApp.Errors do
+  @moduledoc false  # But users need to pattern-match on these!
+
+  defmodule NotFound do
+    defexception [:message]
+  end
+end
+```
+
+**Better alternative:**
+```elixir
+defmodule MyApp.Errors do
+  @moduledoc "Error types raised by MyApp operations."
+
+  defmodule NotFound do
+    @moduledoc "Raised when a resource cannot be found."
+    defexception [:message]
+  end
+end
+```
+
+**Why:** `@moduledoc false` means "this is not for you." If users catch your exceptions or match on your structs, they need documentation. Hide implementation details, not public contracts.''',
+
+    4: '''
+### When to Use
+
+**Triggers:**
+- Your struct has fields that make no sense as `nil` (creating one without them is a bug)
+- You're modeling a value object where all fields define its identity
+- Incomplete structs would cause confusing runtime errors later
+
+**Example — before:**
+```elixir
+defmodule Order do
+  defstruct [:id, :customer_id, :items, :total]
+  # Can create %Order{} with everything nil — meaningless
+end
+```
+
+**Example — after:**
+```elixir
+defmodule Order do
+  @enforce_keys [:customer_id, :items, :total]
+  defstruct [:id | @enforce_keys]
+  # %Order{} without required fields → immediate compile/runtime error
+end
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- The struct is built incrementally (e.g., a changeset or builder pattern)
+- Most fields have sensible defaults
+- The struct represents configuration where partial specs are valid
+
+**Over-application example:**
+```elixir
+# Enforcing keys on a struct that's built in stages
+defmodule FormState do
+  @enforce_keys [:step, :name, :email, :address, :payment]
+  defstruct @enforce_keys
+  # Can't create a partial form state for step 1!
+end
+```
+
+**Better alternative:**
+```elixir
+defmodule FormState do
+  defstruct step: 1, name: nil, email: nil, address: nil, payment: nil
+  # Built incrementally as user progresses through steps
+end
+```
+
+**Why:** `@enforce_keys` is for structs that represent *complete* values. If your struct represents an evolving state or has legitimate intermediate forms, enforcing all keys makes construction impossible at early stages.''',
+
+    5: '''
+### When to Use
+
+**Triggers:**
+- Your `use` macro needs to give the caller access to specific functions
+- You want to control exactly which functions enter the caller's namespace
+- The imported functions are central to the DSL or workflow the module enables
+
+**Example — before:**
+```elixir
+defmacro __using__(_opts) do
+  quote do
+    # Imports EVERYTHING from three modules — namespace soup
+    import MyApp.Router.Helpers
+    import MyApp.Router.Scoping
+    import MyApp.Router.Compilation
+  end
+end
+```
+
+**Example — after:**
+```elixir
+defmacro __using__(_opts) do
+  quote do
+    import MyApp.Router, only: [get: 2, post: 2, resources: 2, scope: 2]
+    import MyApp.Conn, only: [assign: 3, put_status: 2]
+  end
+end
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- The caller could just `import` what they need themselves
+- You're importing utility functions that aren't part of your module's "DSL"
+- The imports create naming conflicts with common functions
+
+**Over-application example:**
+```elixir
+defmacro __using__(_opts) do
+  quote do
+    import MyApp.Utils  # 50+ utility functions dumped into caller
+    import Enum         # Why? Caller can do this themselves
+    import Map          # Polluting namespace with standard lib
+  end
+end
+```
+
+**Better alternative:**
+```elixir
+defmacro __using__(_opts) do
+  quote do
+    # Only import what THIS module's workflow requires
+    import MyApp.DSL, only: [field: 2, validate: 1]
+  end
+end
+```
+
+**Why:** `use` should import the *minimum* needed for the module's intended workflow. If you're importing generic utilities, you're making decisions for the caller that they should make themselves.''',
+
+    6: '''
+### When to Use
+
+**Triggers:**
+- Multiple modules from the same parent namespace are used together
+- Full module paths are making code hard to read
+- The aliased modules are used frequently (3+ times in the file)
+
+**Example — before:**
+```elixir
+def process(input) do
+  Phoenix.Router.Route.new(input)
+  |> Phoenix.Router.Scope.apply_scope(Phoenix.Router.Scope.current())
+  |> Phoenix.Router.Helpers.generate()
+end
+```
+
+**Example — after:**
+```elixir
+alias Phoenix.Router.{Route, Scope, Helpers}
+
+def process(input) do
+  Route.new(input)
+  |> Scope.apply_scope(Scope.current())
+  |> Helpers.generate()
+end
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- A module is referenced only once (inline the full path)
+- The alias would be ambiguous (two `Route` modules from different namespaces)
+- You're in a test file and the full path makes assertions clearer
+
+**Over-application example:**
+```elixir
+# Aliasing a module used exactly once
+alias MyApp.Workers.BatchProcessor
+
+def run do
+  BatchProcessor.start()  # Only reference — alias adds noise
+end
+```
+
+**Better alternative:**
+```elixir
+def run do
+  MyApp.Workers.BatchProcessor.start()  # One use — full path is fine
+end
+```
+
+**Why:** Aliases trade verbosity for indirection. When a module appears once, the full path is documentation. When it appears many times, the alias is readability. Find the crossover point (typically 2-3 uses).''',
+
+    7: '''
+### When to Use
+
+**Triggers:**
+- A struct field stores a boolean value
+- The field answers a yes/no question about the struct
+- You want the field's type to be self-evident without checking typespecs
+
+**Example — before:**
+```elixir
+defstruct [:path, :trailing_slash, :verified]
+# Is :trailing_slash the slash character? A boolean? The position?
+```
+
+**Example — after:**
+```elixir
+defstruct [:path, :trailing_slash?, :verified?]
+# Immediately clear these are booleans
+```
+
+### When NOT to Use
+
+**Don't use this when:**
+- The field isn't a boolean (e.g., `:status` that can be `:active`/`:inactive`)
+- You're working with external serialization that can't handle `?` in keys
+- The field represents a count, enum, or value rather than a yes/no question
+
+**Over-application example:**
+```elixir
+defstruct [:user?, :admin?, :count?]
+# :user? — is this "is user present?" or "the user value"?
+# :count? — definitely not a boolean
+```
+
+**Better alternative:**
+```elixir
+defstruct [:user, :admin?, :count]
+# :user is the user struct, :admin? is a boolean, :count is an integer
+```
+
+**Why:** The `?` suffix should only mark genuine booleans. Using it on non-boolean fields creates confusion about the field's type and breaks the convention's usefulness as a type signal.''',
+}
+
+for i in range(len(parts)):
+    if i in when_sections:
+        parts[i] = parts[i].rstrip() + "\n\n" + when_sections[i].strip()
+
+output = separator.join(parts) + "\n"
+
+with open("patterns/modules.md", "w") as f:
+    f.write(output)
+
+print(f"Done! {len(output)} chars")