Created
November 13, 2025 23:48
-
-
Save timcunningham/6bddf2a278791d53c77ce8979e83e803 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| # ColdFusion Project | |
| This is a ColdFusion (CFML) application. Use modern CFML practices, prioritize security, and optimize performance. | |
| ## Clean Code Principles (Bob Martin) | |
| These principles apply to ALL code you write, regardless of language: | |
| ### Meaningful Names | |
| - **Use intention-revealing names**: Names should answer why it exists, what it does, and how it's used | |
| - **Avoid disinformation**: Don't use names that obscure meaning (e.g., `accountList` when it's not actually a list) | |
| - **Make meaningful distinctions**: Don't use number-series naming (`a1`, `a2`) or noise words (`ProductInfo` vs `ProductData`) | |
| - **Use pronounceable names**: If you can't pronounce it, you can't discuss it intelligently | |
| - **Use searchable names**: Single-letter names and numeric constants are hard to locate across a codebase | |
| - **Avoid encodings**: Don't prefix with type or scope (no Hungarian notation, no `m_` prefixes) | |
| - **Class names**: Should be nouns or noun phrases (`Customer`, `WikiPage`, `Account`) | |
| - **Method names**: Should be verbs or verb phrases (`postPayment`, `deletePage`, `save`) | |
| - **Pick one word per concept**: Don't mix `fetch`, `retrieve`, and `get` for the same concept | |
| - **Use solution domain names**: Use CS terms, algorithm names, pattern names when appropriate | |
| - **Add meaningful context**: Use prefixes/namespaces when needed (`addrFirstName` vs `firstName` when context unclear) | |
| ### Functions | |
| - **Small**: Functions should be small. Then make them smaller. 20 lines is generous, 5-10 lines is ideal | |
| - **Do one thing**: Functions should do ONE thing, do it well, do it only | |
| - **One level of abstraction per function**: Don't mix high-level concepts with low-level details | |
| - **Reading code from top to bottom**: Code should read like a narrative, with descending levels of abstraction | |
| - **Switch statements**: Bury in low-level classes, never repeat. Use polymorphism instead | |
| - **Use descriptive names**: Long descriptive names are better than short enigmatic names | |
| - **Function arguments**: | |
| - Zero arguments (niladic) is ideal | |
| - One argument (monadic) is good | |
| - Two arguments (dyadic) is acceptable but scrutinize | |
| - Three arguments (triadic) should be avoided when possible | |
| - More than three (polyadic) requires special justification | |
| - **Flag arguments are ugly**: Passing a boolean is a terrible practice (function does more than one thing) | |
| - **Argument objects**: When functions need >2-3 arguments, wrap them in a class/object | |
| - **Have no side effects**: Don't have hidden side effects. Don't modify arguments. Don't set unexpected state | |
| - **Command Query Separation**: Functions should either DO something or ANSWER something, never both | |
| - **Prefer exceptions to returning error codes**: Use exceptions for error handling, not return codes | |
| - **Extract try/catch blocks**: Error handling is one thing. Extract the bodies of try/catch into their own functions | |
| - **Don't repeat yourself (DRY)**: Duplication is the root of all evil in software | |
| ### Comments | |
| - **Don't comment bad code—rewrite it**: Comments are a failure to express yourself in code | |
| - **Explain yourself in code**: `if (employee.isEligibleForFullBenefits())` is better than `// Check if eligible` | |
| - **Good comments**: | |
| - Legal comments (copyright, licenses) | |
| - Informative comments (explaining regex patterns, return values) | |
| - Explanation of intent | |
| - Warning of consequences | |
| - TODO comments (but track them) | |
| - Amplification (emphasizing importance of something that might seem inconsequential) | |
| - **Bad comments**: | |
| - Mumbling: If you must comment, make sure it's clear | |
| - Redundant comments: Don't state the obvious | |
| - Misleading comments: Worse than no comment | |
| - Mandated comments: Not every function needs a comment | |
| - Journal comments: Use source control instead | |
| - Noise comments: Restating the obvious with no new information | |
| - Position markers: `// Actions //////////////` | |
| - Closing brace comments: `} // end if` | |
| - Attributions and bylines: Source control remembers who wrote what | |
| - Commented-out code: Delete it. Source control remembers | |
| - HTML comments: Comments should be readable in plain text | |
| - Nonlocal information: Don't put system-wide info in local comment | |
| - Too much information: Don't put historical essays or details in comments | |
| - Inobvious connection: Connection between comment and code should be obvious | |
| ### Formatting | |
| - **The purpose of formatting is communication**: Code formatting is about communication, and communication is the professional developer's first order of business | |
| - **Vertical formatting**: | |
| - Small files are easier to understand than large files (200-500 lines max per file) | |
| - Newspaper metaphor: Name tells you if you're in the right module. Top gives high-level concepts. Detail increases as you move down | |
| - Vertical openness: Blank lines separate concepts | |
| - Vertical density: Lines of code that are tightly related should appear vertically dense | |
| - Vertical distance: Concepts that are closely related should be vertically close | |
| - Variable declarations: As close to their usage as possible | |
| - Instance variables: At the top of the class | |
| - Dependent functions: If one function calls another, they should be vertically close, caller above callee | |
| - Conceptual affinity: Stronger the affinity, less vertical distance | |
| - **Horizontal formatting**: | |
| - Keep lines short (80-120 characters as a guideline) | |
| - Use horizontal white space to associate strongly related things and disassociate weakly related | |
| - Don't align declarations in columns (it emphasizes wrong things) | |
| - Indentation is critical: Never break indentation rules even for short if/while statements | |
| - **Team rules**: Every team should agree on a single formatting style and ALL members should use it | |
| ### Objects and Data Structures | |
| - **Data abstraction**: Hide implementation. Don't just add getters/setters | |
| - **Objects expose behavior and hide data**: This makes it easy to add new objects without changing behaviors | |
| - **Data structures expose data and have no behavior**: This makes it easy to add new behaviors to existing data structures | |
| - **The Law of Demeter**: A method should not invoke methods on objects returned by any of its methods | |
| - Talk to friends, not strangers | |
| - Avoid chains: `a.getB().getC().doSomething()` is bad (train wreck) | |
| - Objects should expose behavior, not their internals | |
| - **Data Transfer Objects (DTOs)**: Simple data structures with public variables and no functions are useful for communicating with databases, sockets, etc. | |
| ### Error Handling | |
| - **Use exceptions rather than return codes**: Exceptions separate error handling from happy path | |
| - **Write your try-catch-finally statement first**: This helps define what the user should expect, no matter what goes wrong | |
| - **Use unchecked exceptions**: Checked exceptions violate Open/Closed Principle (in languages that have them) | |
| - **Provide context with exceptions**: Create informative error messages with operation intent and type of failure | |
| - **Define exception classes in terms of caller's needs**: Wrap third-party APIs so you can switch libraries easily and not be tied to API design choices | |
| - **Don't return null**: Returning null creates work for callers (null checks everywhere) and one missed check causes problems | |
| - **Don't pass null**: Passing null into methods is worse than returning null. Avoid it when possible | |
| ### Boundaries | |
| - **Using third-party code**: Learning tests - write tests to explore the third-party API | |
| - **Clean boundaries**: Wrapping third-party code provides a clear boundary and makes it easier to migrate to different library | |
| - **Code at boundaries needs clear separation**: Avoid letting too much of your code know about the third-party particulars | |
| ### Unit Tests | |
| - **The Three Laws of TDD**: | |
| 1. You may not write production code until you have written a failing unit test | |
| 2. You may not write more of a unit test than is sufficient to fail | |
| 3. You may not write more production code than is sufficient to pass the currently failing test | |
| - **Keeping tests clean**: Test code is just as important as production code | |
| - **Tests enable change**: Without tests, you can't refactor. You can't improve the design | |
| - **Clean tests**: Readability, readability, readability - clarity, simplicity, density of expression | |
| - **One assert per test**: Minimize the number of asserts per test (single concept per test) | |
| - **F.I.R.S.T**: | |
| - **Fast**: Tests should run quickly | |
| - **Independent**: Tests should not depend on each other | |
| - **Repeatable**: Tests should be repeatable in any environment | |
| - **Self-Validating**: Tests should have a boolean output (pass/fail) | |
| - **Timely**: Write tests in a timely fashion (just before production code) | |
| ### Classes | |
| - **Class organization**: Public static constants, private static variables, private instance variables, then public functions, then private utilities called by public functions | |
| - **Classes should be small**: Measured by responsibilities, not lines of code | |
| - **Single Responsibility Principle (SRP)**: A class should have one, and only one, reason to change | |
| - **Cohesion**: Classes should have a small number of instance variables. Methods should manipulate one or more of those variables. High cohesion means methods and variables are co-dependent | |
| - **Maintaining cohesion results in many small classes**: When classes lose cohesion, split them | |
| - **Organizing for change**: Classes should be open for extension but closed for modification (Open-Closed Principle) | |
| - **Isolating from change**: Depend on abstractions, not concretions (Dependency Inversion Principle) | |
| ### Systems | |
| - **Separate constructing a system from using it**: Separate startup process from runtime logic | |
| - **Dependency Injection**: Separate construction from use via DI/IoC containers | |
| - **Scale up**: Software systems are unique compared to physical systems - they can grow incrementally IF we maintain proper separation of concerns | |
| - **Use standards wisely, when they add demonstrable value**: Don't use standards just because they exist | |
| - **Domain-Specific Languages**: Allow domain experts to write code in their domain language | |
| ### Emergence (Simple Design) | |
| Kent Beck's four rules of Simple Design (in priority order): | |
| 1. **Runs all the tests**: A design must produce a system that acts as intended | |
| 2. **Contains no duplication**: Duplication is the enemy of a well-designed system | |
| 3. **Expresses the intent of the programmer**: Choose good names. Keep functions and classes small. Use standard nomenclature | |
| 4. **Minimizes the number of classes and methods**: Keep function, class, and module counts low. Don't be dogmatic about small classes/functions if it doesn't add value | |
| ### Concurrency (Where Applicable) | |
| - **Keep concurrency code separate**: Don't mix concurrency code with other code | |
| - **Limit the scope of data**: Use synchronized/locked blocks to protect critical sections | |
| - **Use copies of data**: Avoid sharing data when possible. Use immutable objects | |
| - **Threads should be as independent as possible**: Attempt to partition data into independent subsets | |
| ### Successive Refinement | |
| - **Make it work, then make it right**: First get it working, then refactor to clean it up | |
| - **Incrementalism**: Software development is iterative and incremental | |
| - **Clean code is not written by following rules**: It's written by laboriously applying principles through successive refinement | |
| ## Common bash commands | |
| - `box server start`: Start the ColdFusion development server | |
| - `box server restart`: Restart the server after config changes | |
| - `box testbox run`: Run all TestBox unit tests | |
| - `box testbox run --bundles tests.specs.UserServiceTest`: Run specific test bundle | |
| - `box install`: Install dependencies from box.json | |
| - `box package show`: Show installed packages | |
| - `cfformat **.cfc --overwrite`: Format all CFC files | |
| - `fixinator .`: Scan for security vulnerabilities | |
| ## Core files and utility functions | |
| - `Application.cfc`: Main application configuration and lifecycle events | |
| - `/components/services/`: Business logic services (UserService.cfc, AuthService.cfc) | |
| - `/components/utils/QueryHelper.cfc`: Database query utilities with parameterization | |
| - `/components/utils/Validator.cfc`: Input validation functions | |
| - `/api/`: REST API endpoints | |
| - `/tests/specs/`: TestBox test specifications | |
| - `box.json`: Package dependencies | |
| - `.cfformat.json`: Code formatting rules | |
| ## Code style | |
| - Use CFScript syntax in .cfc files for business logic, not tags | |
| - Use CFML tags only in .cfm view templates | |
| - Always use queryExecute() with parameters, never concatenate SQL strings | |
| - Use member functions: `myArray.append()` instead of `arrayAppend(myArray)` | |
| - Use safe navigation operator: `user?.profile?.email ?: "default"` | |
| - Destructure where possible: `var { id, name } = getUserData()` | |
| - Prefer `local` scope in functions over `var` declarations | |
| ### Naming conventions (Clean Code) | |
| - **Functions/Methods**: Use verb phrases that reveal intent (`calculateTotal`, `validateEmail`, `retrieveCustomer`) | |
| - **Variables**: Use meaningful nouns that are searchable and pronounceable (`invoiceDate`, not `id` or `dt`) | |
| - **Constants**: Use UPPERCASE_WITH_UNDERSCORES for true constants | |
| - **Boolean variables**: Use `is`, `has`, `can` prefixes (`isValid`, `hasPermission`, `canEdit`) | |
| - **No abbreviations**: Write `customer` not `cust`, `message` not `msg` (unless domain-specific like `HTTP`, `URL`) | |
| - **One word per concept**: Pick `get`, `fetch`, or `retrieve` and stick with it across the codebase | |
| - **Avoid mental mapping**: Readers shouldn't have to mentally translate your names into other names they already know | |
| ### Function design (Clean Code) | |
| - **Keep functions small**: Aim for 5-10 lines, maximum 20 lines | |
| - **Do one thing only**: If a function does multiple things, split it up | |
| - **One level of abstraction**: Don't mix high-level logic with low-level details in same function | |
| - **Minimize arguments**: 0-1 arguments ideal, 2 acceptable, 3+ needs refactoring into an object | |
| - **No flag arguments**: Never pass booleans to indicate behavior. Split into two functions instead | |
| - **No side effects**: Functions should not modify global state, arguments, or have hidden behaviors | |
| - **Command-Query Separation**: Functions either change state OR return information, never both | |
| - **Extract try/catch bodies**: Error handling is one thing. Put try/catch bodies in separate functions | |
| - **DRY principle**: If you write the same code twice, extract it into a function | |
| ## Testing instructions | |
| ### TDD Workflow (Clean Code) | |
| - **The Three Laws of TDD**: | |
| 1. Write a failing test before any production code | |
| 2. Write only enough of a test to demonstrate a failure | |
| 3. Write only enough production code to pass the failing test | |
| - **Tests are first-class citizens**: Test code is as important as production code | |
| - **F.I.R.S.T Principles**: | |
| - **Fast**: Tests must run quickly (milliseconds, not seconds) | |
| - **Independent**: Tests should not depend on each other or run in specific order | |
| - **Repeatable**: Must work in any environment (dev, staging, prod, offline) | |
| - **Self-Validating**: Boolean output (pass/fail), no manual inspection | |
| - **Timely**: Write tests just before the production code that makes them pass | |
| ### Test practices | |
| - Test files go in `/tests/specs/` with `Test` suffix (UserServiceTest.cfc) | |
| - Use BDD style with TestBox: `describe()` and `it()` blocks | |
| - Mock external dependencies with MockBox | |
| - Run specific test: `box testbox run --bundles tests.specs.UserServiceTest` | |
| - **One concept per test**: Each test should verify one specific behavior | |
| - **Minimize asserts**: Ideally one assert per test, maximum 2-3 related asserts | |
| - **Test readability matters**: Use BUILD-OPERATE-CHECK pattern (Given-When-Then) | |
| - **Test coverage**: Minimum 80% for business logic, 100% for critical paths | |
| - **Test edge cases**: Empty inputs, null values, boundary conditions, error states | |
| - **Don't test getters/setters**: Test behavior, not simple property access | |
| ## Repository etiquette | |
| - Branch naming: `feature/description`, `fix/issue-number`, `hotfix/description` | |
| - Commit messages: Use conventional commits (feat:, fix:, docs:, test:) | |
| - Always create feature branches from `develop`, never from `main` | |
| - Squash commits before merging | |
| - Run tests and linting before committing | |
| - Update CHANGELOG.md for user-facing changes | |
| ## Developer environment setup | |
| - ColdFusion 2023/2024 or Lucee 6.x | |
| - CommandBox CLI required (`brew install commandbox` on Mac) | |
| - VS Code with CFML extension or Adobe ColdFusion Builder | |
| - Configure datasource in `.env` file (copy from `.env.example`) | |
| - Run `box install` to get dependencies | |
| - Run `box server start` to launch development server on port 8500 | |
| - FusionReactor for performance monitoring (optional) | |
| ## Unexpected behaviors or warnings | |
| - Arrays pass by VALUE in Adobe ColdFusion (unlike Java/Lucee) - use duplicate() if needed | |
| - `cfcookie` without `secure` attribute will fail in production (HTTPS required) | |
| - Query of queries (QoQ) is case-sensitive unlike regular SQL queries | |
| - `serializeJSON()` converts strings that look like numbers - use struct notation | |
| - ColdFusion caches CFC metadata - restart server after adding cfproperty tags | |
| - URL/FORM scopes are case-insensitive but other structs are case-sensitive | |
| - `isNull()` doesn't work with all null types - also check `!isDefined()` or `!structKeyExists()` | |
| ## Security requirements | |
| - ALWAYS use cfqueryparam or queryExecute with parameters for SQL | |
| - ALWAYS encode output: `encodeForHTML()`, `encodeForJavaScript()`, `encodeForURL()` | |
| - NEVER use evaluate() - use bracket notation instead: `form["field#i#"]` | |
| - NEVER store passwords as plain text - use bcrypt or argon2 | |
| - NEVER expose detailed error messages in production | |
| - Validate file uploads: check type, size, and scan for malware | |
| - Set secure and httpOnly flags on cookies | |
| - Implement CSRF tokens for state-changing operations | |
| ## Formatting and File Organization (Clean Code) | |
| ### File size and structure | |
| - **Small files**: Keep files under 200-500 lines maximum | |
| - **Newspaper metaphor**: File name tells you if you're in the right place. Top has high-level concepts. Detail increases as you go down | |
| - **Vertical openness**: Use blank lines to separate different concepts | |
| - **Vertical density**: Keep tightly related code close together without blank lines | |
| - **Vertical distance**: Related concepts should be vertically close | |
| - Variable declarations close to usage | |
| - Private functions right after the public function that calls them | |
| - Caller above callee when possible | |
| ### Class/Component organization order | |
| 1. Public static constants | |
| 2. Private static variables | |
| 3. Private instance variables | |
| 4. Public constructor | |
| 5. Public functions | |
| 6. Private functions (grouped near the public functions that call them) | |
| ### Horizontal formatting | |
| - **Line length**: Keep lines under 80-120 characters | |
| - **Indentation is sacred**: Never skip indentation, even for one-line if/while statements | |
| - **Horizontal whitespace**: Use spaces to show relationships | |
| - Spaces around operators: `a + b`, not `a+b` | |
| - No space between function name and parenthesis: `getValue()`, not `getValue ()` | |
| - Space after commas: `foo(a, b, c)`, not `foo(a,b,c)` | |
| ### Team formatting rules | |
| - Define formatting rules once and follow them consistently | |
| - Use automated formatters (cfformat for CFML) | |
| - The team's formatting standard > your personal preference | |
| ## Refactoring and Code Quality | |
| ### When to refactor | |
| - **Boy Scout Rule**: Leave code cleaner than you found it | |
| - **Before adding features**: Clean up existing code first to make new feature easier | |
| - **During code review**: If code is hard to understand, refactor before merging | |
| - **When you notice smells**: Long functions, duplicate code, poor names, etc. | |
| ### Refactoring practices | |
| - **Make it work, then make it right**: Get tests passing first, then refactor with tests protecting you | |
| - **Small steps**: Refactor in small increments, running tests after each change | |
| - **Extract functions**: When you see a block of code doing something specific, extract it to a named function | |
| - **Extract constants**: Replace magic numbers and strings with named constants | |
| - **Rename for clarity**: Spend time finding the right name. Clear names are worth it | |
| - **Reduce function arguments**: If function takes >2 arguments, consider grouping them into an object | |
| ### Code smells to eliminate | |
| - **Long functions**: Break into smaller functions (max 20 lines) | |
| - **Large classes**: Classes with too many responsibilities need splitting (SRP) | |
| - **Long parameter lists**: More than 2-3 parameters suggests missing abstraction | |
| - **Duplicate code**: Extract common code into functions | |
| - **Dead code**: Delete unused functions, variables, parameters | |
| - **Speculative generality**: Don't add functionality "just in case" - YAGNI (You Ain't Gonna Need It) | |
| - **Comments**: Often indicate code that needs better naming/structure | |
| - **Inconsistent naming**: Same concept should use same name throughout codebase | |
| ### SOLID Principles (for classes/components) | |
| - **Single Responsibility (SRP)**: A class should have one reason to change | |
| - **Open/Closed (OCP)**: Open for extension, closed for modification | |
| - **Liskov Substitution (LSP)**: Subtypes must be substitutable for base types | |
| - **Interface Segregation (ISP)**: Many specific interfaces better than one general interface | |
| - **Dependency Inversion (DIP)**: Depend on abstractions, not concretions | |
| ## Common workflows | |
| ### Fix a bug | |
| 1. Read the issue/bug report | |
| 2. Write a failing test that reproduces the bug | |
| 3. Run the test to confirm it fails | |
| 4. Fix the code | |
| 5. Run the test to confirm it passes | |
| 6. Run full test suite to ensure no regressions | |
| 7. Commit with message: `fix: description (#issue-number)` | |
| ### Add a new REST endpoint | |
| 1. Read existing API examples in `/api/v1/` | |
| 2. Create new CFC with `rest="true"` and appropriate `restpath` | |
| 3. Write integration tests first | |
| 4. Implement the endpoint methods | |
| 5. Test with: `curl -X GET http://localhost:8500/rest/api/v1/resource` | |
| 6. Update API documentation | |
| ### Database migration | |
| 1. Create migration file in `/migrations/` with timestamp prefix | |
| 2. Write UP and DOWN SQL scripts | |
| 3. Test migration locally: `box migrate up` | |
| 4. Test rollback: `box migrate down` | |
| 5. Commit migration file | |
| ## Performance tips | |
| - Cache expensive operations in application scope | |
| - Use query caching for static data: `cachedwithin="#createTimeSpan(0,1,0,0)#"` | |
| - Avoid SELECT * - specify needed columns | |
| - Index foreign keys and WHERE clause columns | |
| - Batch database operations when possible | |
| - Use streaming for large datasets | |
| - Monitor with FusionReactor in production | |
| ## Simple Design Principles (Kent Beck) | |
| Apply these four rules in priority order: | |
| 1. **Runs all tests**: The design must produce a system that works as intended | |
| 2. **Contains no duplication**: Eliminate duplicate code through extraction and abstraction | |
| 3. **Expresses the intent of the programmer**: Code should clearly communicate what it does | |
| 4. **Minimizes classes and methods**: Keep your design lean, but don't sacrifice clarity | |
| ### Applying Simple Design | |
| - Don't over-engineer: Solve today's problem, not tomorrow's imagined problem (YAGNI) | |
| - Duplication is the enemy: Every piece of knowledge should have one authoritative representation | |
| - Express intent through naming: Names should make the code read like well-written prose | |
| - Keep it minimal: Fewer moving parts mean fewer bugs and easier maintenance | |
| ## API patterns | |
| ### Clean REST endpoint (following Clean Code principles) | |
| ```cfscript | |
| component rest="true" restpath="/users" { | |
| property name="userService" inject="UserService"; | |
| property name="validator" inject="Validator"; | |
| property name="logger" inject="Logger"; | |
| // Clean: Descriptive name, single responsibility, minimal code | |
| remote function getUser(required numeric id restargsource="Path") | |
| httpmethod="GET" restpath="{id}" produces="application/json" { | |
| try { | |
| return retrieveUserById(arguments.id); | |
| } catch (UserNotFoundException e) { | |
| return buildNotFoundResponse(e.message); | |
| } catch (ValidationException e) { | |
| return buildBadRequestResponse(e.message); | |
| } catch (any e) { | |
| logger.error("Error retrieving user", e); | |
| return buildServerErrorResponse(); | |
| } | |
| } | |
| // Extracted for clarity - single level of abstraction | |
| private function retrieveUserById(required numeric id) { | |
| validateUserId(id); | |
| return userService.findById(id); | |
| } | |
| // Named validation - intention revealing | |
| private function validateUserId(required numeric id) { | |
| if (id <= 0) { | |
| throw(type="ValidationException", message="User ID must be positive"); | |
| } | |
| } | |
| // Consistent response builders - DRY principle | |
| private function buildNotFoundResponse(required string message) { | |
| restSetResponse({status: 404}); | |
| return {error: message}; | |
| } | |
| private function buildBadRequestResponse(required string message) { | |
| restSetResponse({status: 400}); | |
| return {error: message}; | |
| } | |
| private function buildServerErrorResponse() { | |
| restSetResponse({status: 500}); | |
| return {error: "An unexpected error occurred"}; | |
| } | |
| } | |
| ``` | |
| ### Contrasting example: Before and After Clean Code | |
| ```cfscript | |
| // BEFORE: Violates multiple Clean Code principles | |
| component { | |
| remote function process(required struct data) { | |
| var result = {}; | |
| // Validate | |
| if (!structKeyExists(data, "email") || !len(trim(data.email))) { | |
| result.error = "Email required"; | |
| result.code = 400; | |
| return result; | |
| } | |
| // Check format | |
| if (!reFind("^[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}$", data.email)) { | |
| result.error = "Invalid email"; | |
| result.code = 400; | |
| return result; | |
| } | |
| // Get user | |
| var q = queryExecute("SELECT * FROM users WHERE email = '" & data.email & "'"); | |
| if (q.recordCount == 0) { | |
| result.error = "Not found"; | |
| result.code = 404; | |
| return result; | |
| } | |
| // Update | |
| queryExecute("UPDATE users SET status = 1 WHERE email = '" & data.email & "'"); | |
| result.success = true; | |
| result.code = 200; | |
| return result; | |
| } | |
| } | |
| // AFTER: Follows Clean Code principles | |
| component { | |
| property name="userService" inject="UserService"; | |
| property name="emailValidator" inject="EmailValidator"; | |
| // Clear intent, single responsibility | |
| remote function activateUserAccount(required string email) { | |
| try { | |
| validateEmail(email); | |
| var user = retrieveUserByEmail(email); | |
| activateUser(user); | |
| return buildSuccessResponse(); | |
| } catch (ValidationException e) { | |
| return buildErrorResponse(400, e.message); | |
| } catch (UserNotFoundException e) { | |
| return buildErrorResponse(404, e.message); | |
| } | |
| } | |
| // Small functions, one thing each | |
| private function validateEmail(required string email) { | |
| if (!len(trim(email))) { | |
| throw(type="ValidationException", message="Email is required"); | |
| } | |
| if (!emailValidator.isValid(email)) { | |
| throw(type="ValidationException", message="Email format is invalid"); | |
| } | |
| } | |
| private function retrieveUserByEmail(required string email) { | |
| var user = userService.findByEmail(email); | |
| if (isNull(user)) { | |
| throw(type="UserNotFoundException", message="User not found"); | |
| } | |
| return user; | |
| } | |
| private function activateUser(required struct user) { | |
| userService.activate(user.id); | |
| } | |
| private function buildSuccessResponse() { | |
| return {success: true, code: 200}; | |
| } | |
| private function buildErrorResponse(required numeric code, required string message) { | |
| return {error: message, code: code}; | |
| } | |
| } | |
| ``` | |
| ## Error handling (Clean Code) | |
| ### Principles | |
| - **Prefer exceptions over error codes**: Separates error handling from happy path | |
| - **Don't return null**: Forces null checks everywhere. Return empty objects, empty arrays, or throw exceptions | |
| - **Don't pass null**: Passing null as argument is even worse. Avoid when possible | |
| - **Provide context**: Exception messages should explain the operation attempted and failure type | |
| - **Wrap third-party APIs**: Create your own exception types, don't let third-party exceptions leak into your code | |
| - **Define exceptions by caller's needs**: Create exception classes based on how they'll be caught, not how they're thrown | |
| ### Error handling template | |
| ```cfscript | |
| // Extract try/catch body into separate function (Clean Code principle) | |
| function processPayment(required struct paymentData) { | |
| try { | |
| return executePaymentTransaction(paymentData); | |
| } catch (Database e) { | |
| logError(e, "Database error during payment processing"); | |
| throw(type="PaymentException", message="Unable to process payment. Please try again."); | |
| } catch (ValidationException e) { | |
| logError(e, "Payment validation failed"); | |
| throw(type="PaymentException", message=e.message); | |
| } catch (any e) { | |
| logError(e, "Unexpected error during payment processing"); | |
| throw(type="PaymentException", message="An unexpected error occurred. Support has been notified."); | |
| } | |
| } | |
| // Actual work is in separate function (Single Responsibility) | |
| private function executePaymentTransaction(required struct paymentData) { | |
| validatePaymentData(paymentData); | |
| var transaction = createTransaction(paymentData); | |
| var response = callPaymentGateway(transaction); | |
| return processGatewayResponse(response); | |
| } | |
| // Helper for consistent error logging | |
| private function logError(required any exception, required string context) { | |
| writeLog( | |
| type="error", | |
| file="application", | |
| text="#context#: #serializeJSON(exception)#" | |
| ); | |
| } | |
| ``` | |
| ### Anti-patterns to avoid | |
| ```cfscript | |
| // BAD: Returning null forces null checks everywhere | |
| function getUser(id) { | |
| var user = queryExecute("SELECT * FROM users WHERE id = :id", {id: id}); | |
| return user.recordCount ? user : null; // DON'T DO THIS | |
| } | |
| // GOOD: Return empty result or throw exception | |
| function getUser(required numeric id) { | |
| var user = queryExecute( | |
| "SELECT * FROM users WHERE id = :id", | |
| {id: {value: id, cfsqltype: "cf_sql_integer"}} | |
| ); | |
| if (!user.recordCount) { | |
| throw(type="UserNotFoundException", message="User with ID #id# not found"); | |
| } | |
| return user; | |
| } | |
| // BAD: Generic catch-all with no context | |
| try { | |
| doSomething(); | |
| } catch (any e) { | |
| // Silent failure or generic error | |
| } | |
| // GOOD: Specific catches with context | |
| try { | |
| updateUserProfile(userId, profileData); | |
| } catch (ValidationException e) { | |
| throw(type="ProfileUpdateException", message="Profile validation failed: #e.message#"); | |
| } catch (Database e) { | |
| logError(e, "Database error updating user profile for userId: #userId#"); | |
| throw(type="ProfileUpdateException", message="Unable to save profile changes"); | |
| } | |
| ``` | |
| ## Comments (Clean Code) | |
| ### Philosophy | |
| - **Comments are failures**: If you need a comment, the code isn't clear enough | |
| - **Refactor instead**: Extract to well-named functions rather than commenting | |
| - **Code is the truth**: Comments lie over time as code changes but comments don't | |
| ### Good comments (rare cases) | |
| - **Legal comments**: Copyright, licenses (keep at top of file) | |
| - **Warning of consequences**: "This takes 3 hours to run", "Changing this breaks integration" | |
| - **TODO comments**: Must be tracked in issue system, not permanent | |
| - **Amplification**: Explain why something seemingly trivial is actually important | |
| - **Public API documentation**: For libraries, explain parameters, return values, exceptions | |
| ### Bad comments (delete these) | |
| - **Redundant**: `i++; // increment i` - code already says this | |
| - **Misleading**: Comments that don't match what code does | |
| - **Noise**: `// Constructor`, `// Default constructor`, `// Returns the value` | |
| - **Commented-out code**: DELETE IT. Git remembers everything | |
| - **Position markers**: `///////////// Actions /////////////` | |
| - **Closing brace comments**: `} // end if` `} // end for` | |
| - **Attributions**: `// Added by John` - Git knows who wrote what | |
| - **Journal comments**: `// 2023-01-15: Fixed bug` - Git has the history | |
| - **Too much information**: Long explanations that should be in documentation | |
| ### Examples | |
| ```cfscript | |
| // BAD: Redundant comment | |
| // Check if user is active | |
| if (user.isActive) { | |
| // Do something | |
| } | |
| // GOOD: Self-explanatory code (no comment needed) | |
| if (user.isActive) { | |
| enableUserFeatures(user); | |
| } | |
| // BAD: Comment explains bad code | |
| // Check if payment is valid (status 1=pending, 2=complete, 3=failed) | |
| if (payment.status == 2) { | |
| processRefund(); | |
| } | |
| // GOOD: Named constants make code self-documenting | |
| const PAYMENT_STATUS_COMPLETE = 2; | |
| if (payment.status == PAYMENT_STATUS_COMPLETE) { | |
| processRefund(); | |
| } | |
| // EVEN BETTER: Extract to intention-revealing function | |
| if (isPaymentComplete(payment)) { | |
| processRefund(); | |
| } | |
| // ACCEPTABLE: Warning of consequences | |
| // WARNING: This query can take 30+ seconds on production database | |
| // Consider running during off-peak hours | |
| var report = generateAnnualReport(); | |
| // ACCEPTABLE: Explanation of intent for non-obvious business rule | |
| // Per IRS regulations, we must retain records for 7 years | |
| var retentionPeriodDays = 365 * 7; | |
| ``` |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment