code-quality

# Code Quality — Implementation Guide Covers: **Metrics · Static Analysis · Testing · Technical Debt · Code Review · Automation** ----- ## Code Quality Metrics ### Complexity Metrics Understanding and tracking complexity metrics helps identify problematic code that may be difficult to maintain, test, or extend. Different metrics provide different insights into code quality. **Cyclomatic Complexity** measures the number of linearly independent paths through code. Higher values indicate more decision points and increased testing difficulty. A function with complexity above 10 becomes difficult to test thoroughly, while complexity above 20 indicates urgent refactoring need. **Cognitive Complexity** measures how difficult code is to understand conceptually. Unlike cyclomatic complexity, cognitive complexity accounts for nested structures and logical constructs that make code harder for humans to parse mentally. This metric aligns better with actual maintainability. **Coupling** measures dependencies between modules. High coupling means changes in one module require changes in others, increasing maintenance burden. Aim for loose coupling with clear interfaces. **Cohesion** measures how related the responsibilities of a single module are. High cohesion (single responsibility) makes code easier to understand, test, and maintain. | Metric | Good | Warning | Critical | |--------|------|---------|----------| | Cyclomatic Complexity | < 10 | 10-20 | > 20 | | Cognitive Complexity | < 15 | 15-30 | > 30 | | Lines per Function | < 20 | 20-50 | > 50 | | Parameters per Function | < 3 | 3-5 | > 5 | | Coupling | < 5 | 5-10 | > 10 | ### Code Analysis Tools ```python # Python example with multiple tools # 1. pylint - Comprehensive Python linter # Configuration: .pylintrc [MESSAGES CONTROL] disable=C0111,C0103,R0903,R0913 [FORMAT] max-line-length=100 indent-string=' ' [DESIGN] max-args=5 max-attributes=7 max-branches=12 max-locals=15 min-public-methods=2 [COMPLEXITY] disable=too-complex # Run: pylint mymodule.py --output-format=text # 2. ruff - Fast Python linter (written in Rust) # Configuration: pyproject.toml [tool.ruff] line-length = 100 target-version = "py311" [tool.ruff.lint] select = [ "E", # pycodestyle errors "W", # pycodestyle warnings "F", # pyflakes "I", # isort "N", # pep8-naming "UP", # pyupgrade "B", # flake8-bugbear "C4", # flake8-comprehensions "SIM", # flake8-simplify ] ignore = [ "E501", # line too long (handled by formatter) "B008", # do not perform function calls in argument defaults ] # Run: ruff check mymodule.py # 3. mypy - Static type checker # Configuration: mypy.ini [mypy] python_version = 3.11 warn_return_any = True warn_unused_configs = True disallow_untyped_defs = False disallow_incomplete_defs = True check_untyped_defs = True no_implicit_optional = True warn_redundant_casts = True warn_unused_ignores = True warn_no_return = True warn_unreachable = True [mypy-tests.*] disallow_untyped_defs = False # Run: mypy mymodule.py ``` ### JavaScript/TypeScript Analysis ```javascript // ESLint configuration: .eslintrc.js module.exports = { root: true, env: { browser: true, es2022: true, node: true, }, extends: [ 'eslint:recommended', 'plugin:@typescript-eslint/recommended', 'plugin:react/recommended', 'plugin:react-hooks/recommended', ], parser: '@typescript-eslint/parser', parserOptions: { ecmaVersion: 'latest', sourceType: 'module', ecmaFeatures: { jsx: true, }, }, plugins: ['@typescript-eslint', 'react', 'react-hooks'], rules: { // TypeScript rules '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], '@typescript-eslint/explicit-function-return-type': 'warn', '@typescript-eslint/explicit-module-boundary-types': 'off', '@typescript-eslint/no-explicit-any': 'warn', // React rules 'react/react-in-jsx-scope': 'off', 'react/prop-types': 'off', // Use TypeScript instead 'react-hooks/rules-of-hooks': 'error', 'react-hooks/exhaustive-deps': 'warn', }, settings: { react: { version: 'detect', }, }, }; // Prettier integration in ESLint // npm install --save-dev eslint-config-prettier // Add to extends: 'prettier' ``` ----- ## Technical Debt ### Identifying and Tracking Technical debt accumulates when teams take shortcuts that create future maintenance burden. Understanding, tracking, and systematically addressing technical debt is essential for long-term project health. **Common Sources of Technical Debt:** - Duplicated code across the codebase - Missing or inadequate documentation - Complex conditional logic without clear structure - Tight coupling between components - Lack of automated tests - Using deprecated libraries or patterns - Hardcoded configuration values - Inconsistent naming or coding standards ### Debt Tracking Tools ```yaml # SonarQube quality gates configuration # sonar-project.properties sonar.projectKey=my-project sonar.projectName=My Project sonar.projectVersion=1.0 sonar.sources=src sonar.tests=tests sonar.sourceEncoding=UTF-8 # Quality gates thresholds sonar.qualitygate.wait=true # Quality profiles sonar.technicalDebt.qualityProfiles=rule:python:S1130 # Issue tracking sonar.issue.ignore.allFile=true # Exclusions sonar.exclusions=**/*.test.js,**/node_modules/**,**/dist/** ``` ```json // GitHub issue labels for technical debt { "labels": [ { "name": "tech-debt", "color": "FC4C02", "description": "Technical debt that needs addressing" }, { "name": "refactoring", "color": "A2EEEF", "description": "Code refactoring task" }, { "name": "performance", "color": "D93F0B", "description": "Performance improvement" }, { "name": "security", "color": "B60205", "description": "Security improvement" } ] } ``` ### Debt Prioritization Matrix | Impact \ Effort | Low | Medium | High | |-----------------|-----|--------|------| | **High** | Quick wins | Plan this quarter | Strategic initiative | | **Medium** | When available | Schedule for later | Define scope | | **Low** | Backlog | Backlog | Maybe never | ----- ## Testing Strategies ### Test Pyramid The test pyramid provides a framework for structuring automated tests. At the base are numerous fast unit tests, followed by fewer integration tests, and finally a small number of slow end-to-end tests. This structure provides fast feedback while maintaining good coverage. **Unit Tests** test individual functions, methods, or classes in isolation. They should be fast, isolated, and test one thing. Mock or stub all external dependencies. Target 70-80% of your test coverage with unit tests. **Integration Tests** test how components work together. These might test database operations, API calls, or interaction between multiple services. They are slower than unit tests but catch integration issues. **End-to-End Tests** test complete user workflows from the UI or API layer. They are slow and brittle but verify the entire system works correctly. Keep these to a minimum. ```python # Example: Unit test with pytest import pytest from unittest.mock import Mock, patch # Source code to test def calculate_discount(price: float, discount_percent: float, member: bool = False) -> float: """Calculate final price with discount""" if price <= 0: raise ValueError("Price must be positive") if discount_percent < 0 or discount_percent > 100: raise ValueError("Discount must be between 0 and 100") discount = price * (discount_percent / 100) # Members get extra 10% if member: discount += price * 0.1 return round(price - discount, 2) class TestCalculateDiscount: """Unit tests for calculate_discount""" def test_basic_discount(self): """Test basic discount calculation""" result = calculate_discount(100, 10) assert result == 90.00 def test_zero_discount(self): """Test no discount applied""" result = calculate_discount(50, 0) assert result == 50.00 def test_full_discount(self): """Test 100% discount""" result = calculate_discount(100, 100) assert result == 0.00 def test_member_discount(self): """Test member gets extra 10%""" result = calculate_discount(100, 10, member=True) assert result == 80.00 # 10% + 10% member = 20% off def test_invalid_price(self): """Test negative price raises error""" with pytest.raises(ValueError, match="Price must be positive"): calculate_discount(-10, 10) def test_invalid_discount(self): """Test discount out of range""" with pytest.raises(ValueError, match="Discount must be between 0 and 100"): calculate_discount(100, 150) @pytest.mark.parametrize("price,discount,expected", [ (100, 10, 90.00), (50, 20, 40.00), (200, 25, 150.00), ]) def test_various_discounts(self, price, discount, expected): """Parametrized test for various scenarios""" result = calculate_discount(price, discount) assert result == expected @patch('__main__.get_member_discount') def test_member_discount_from_api(self, mock_get_discount): """Test member discount fetched from API""" mock_get_discount.return_value = 0.15 # Implementation would call external service # mock_get_discount.assert_called_once() ``` ### Test Coverage ```python # pytest configuration: pytest.ini [pytest] testpaths = tests python_files = test_*.py python_classes = Test* python_functions = test_* addopts = -v --strict-markers --tb=short --cov=src --cov-report=html --cov-report=term-missing:skip-covered --cov-fail-under=80 markers = slow: marks tests as slow (deselect with '-m "not slow"') integration: marks tests as integration: marks tests as unit tests # tests unit Coverage configuration: .coveragerc [run] source = src omit = */tests/* */test_*.py */__init__.py [report] precision = 2 show_missing = True skip_covered = False exclude_lines = pragma: no cover def __repr__ raise AssertionError raise NotImplementedError if __name__ == .__main__.: if TYPE_CHECKING: @abstractmethod ``` ----- ## Code Review ### Guidelines for Effective Reviews Code review is one of the most effective ways to improve code quality and share knowledge across teams. Effective reviews require clear standards, constructive feedback, and balanced effort between authors and reviewers. **For Reviewers:** - Review within 24-48 hours to avoid blocking progress - Focus on substantive issues, not style preferences (use linters) - Ask questions rather than making demands - Suggest solutions, don't just point out problems - Praise good code, not just criticize - Be consistent in what you look for **For Authors:** - Keep PRs small (under 400 lines) - Write clear descriptions explaining the what and why - Self-review before requesting others - Respond to feedback constructively - Don't take feedback personally ### Review Checklist ```markdown # Code Review Checklist ## Correctness - [ ] Does the code work as intended? - [ ] Are edge cases handled properly? - [ ] Are there any potential bugs? - [ ] Is the logic sound? ## Design - [ ] Does the code follow project conventions? - [ ] Is the design appropriate for the problem? - [ ] Is there unnecessary complexity? - [ ] Could the code be simpler? ## Functionality - [ ] Does this match the requirements? - [ ] Are there any missing features? - [ ] Are error cases handled? - [ ] Does it handle concurrent access if applicable? ## Testing - [ ] Are there adequate tests? - [ ] Do tests cover edge cases? - [ ] Are tests maintainable? - [ ] Are integration tests included where needed? ## Security - [ ] Are there any security vulnerabilities? - [ ] Is sensitive data handled properly? - [ ] Are inputs validated? - [ ] Are secrets properly managed? ## Performance - [ ] Are there obvious performance issues? - [ ] Are there unnecessary database calls? - [ ] Is caching considered where appropriate? - [ ] Are large data sets handled efficiently? ## Readability - [ ] Is the code clear and understandable? - [ ] Are names descriptive? - [ ] Are there helpful comments for complex logic? - [ ] Is the code properly formatted? ## Documentation - [ ] Is the public API documented? - [ ] Are complex algorithms explained? - [ ] Are configuration requirements documented? - [ ] Is there a CHANGELOG entry if needed? ``` ### Pre-commit Hooks ```yaml # .pre-commit-config.yaml repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.5.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer - id: check-yaml - id: check-added-large-files args: ['--maxkb=1000'] - id: check-merge-conflict - id: check-toml - id: debug-statements