- Add scripts for generating booking data, building metadata, and updating holidays - Implement public_holidays.py service with holiday lookup functionality - Update CSV data with expanded hotel booking records - Add holidays.json dataset for public holiday dates - Enhance LLM report service with improved formatting - Update metadata domain model and API dependencies - Add test coverage for dataset and public holidays
9.0 KiB
Classes vs. functions: why each file is shaped the way it is
The brief asked for a class-based project. That rule is applied wherever a file actually has state (constructor arguments it reuses) or is one of several interchangeable implementations of the same contract. Where a file is one-shot, stateless glue code that a framework (FastAPI) expects in a specific shape, it is left as a plain function - wrapping it in a class would just be a single-method class, i.e. a function wearing a costume.
Rule of thumb used throughout:
- Class - has constructor state it reuses across methods, groups several related private steps behind one public operation, or is one of multiple interchangeable implementations of an abstract contract (repositories).
- Function - a single, stateless piece of wiring/glue, especially where the framework itself expects a plain callable (FastAPI route handlers, dependency providers).
This mirrors Clean Architecture's layering: the inner layers (domain, services, repositories) hold the real behavior and are classes; the outermost layer (API/framework glue) is intentionally thin and written the way FastAPI wants it - plain functions.
Domain layer
src/nf_hotel_api/domain/schemas.py - classes: Booking, BookingBatch, ReportResponse
Pydantic BaseModel subclasses. This is not a style choice: Pydantic
requires a class to generate field validation, JSON (de)serialization, and
the OpenAPI schema FastAPI exposes at /docs. Each class is a data
contract, not behavior - no methods, just typed fields.
src/nf_hotel_api/domain/metadata.py - classes: RoomType, Address, NearbyEvent, HotelMetadata
Pydantic models for the reference data in data/hotel_metadata.json: address,
per room type its size, standard price per night and number of rooms, plus the
nearby Phnom Penh events (date ranges are validated). Public holidays are not
stored; see services/public_holidays.py. HotelMetadata.describe() renders it all as text for the LLM prompt.
Core (config & security)
src/nf_hotel_api/core/config.py - class Settings + function get_settings()
Settings is a pydantic-settings BaseSettings subclass - again required
by the library to get typed, validated configuration loaded from the
environment / .env. get_settings() is a two-line factory wrapped in
@lru_cache so the Settings object is built once and reused; a function
is all that's needed to memoize a constructor call.
src/nf_hotel_api/core/security.py - class ApiKeyAuthenticator
Implemented as a class with __call__ so a single instance
(require_api_key = ApiKeyAuthenticator()) can be reused as a FastAPI
dependency across every protected route. Being a class also makes it
trivial to unit test in isolation and to extend later (e.g. multiple valid
keys, per-key rate limiting) without touching every route that depends on
it.
Repository layer
src/nf_hotel_api/repositories/metadata_repository.py - class JsonHotelMetadataRepository
Loads and validates hotel_metadata.json into a HotelMetadata.
src/nf_hotel_api/repositories/booking_repository.py - classes: BookingRepository (ABC), CsvBookingRepository, JsonBookingRepository
The textbook case for classes: an abstract base class defines one contract
(load() -> DataFrame), and two concrete classes implement it against
different sources while holding their own state (a file path + separator,
or a list of JSON records). Every service depends only on the abstract
BookingRepository type, so the CSV source can be swapped for the JSON
source (or a future database-backed repository) without touching any
business logic. Plain functions can't express "two interchangeable
implementations of one contract" this cleanly - that's exactly what
classes + inheritance are for.
Services (business logic)
src/nf_hotel_api/services/cleaning.py - class DataCleaningService
Groups five related private steps (_fix_wrong_format,
_clean_empty_cells, _fix_wrong_data, _remove_duplicates, _add_pricing)
behind one public clean() method. It is constructed with a HotelMetadata;
_add_pricing keeps the price paid in the CSV, fills missing prices from the
standard price, and derives room_size and revenue. The class keeps these steps cohesive, individually
testable (see tests/test_cleaning.py), and lets the whole pipeline be
swapped out in ReportService.
src/nf_hotel_api/services/public_holidays.py - class PublicHolidayCalendar
Thin wrapper over the holidays package (country KH): returns holidays per
year, looks up a single day, merges consecutive days into HolidayPeriods and
renders them as text for the LLM prompt. data/holidays.json is an export of
the periods, regenerated by scripts/update_holidays.py.
src/nf_hotel_api/services/statistics.py - class DescriptiveStatsService
A single-purpose class computing and JSON-serializing df.describe(). It
holds no state today, but is a class for consistency with its sibling
services and so it can be constructor-injected into ReportService the
same way they are - adding config later (e.g. which percentiles to include)
won't change any call sites.
src/nf_hotel_api/services/llm_report.py - classes LLMReportService, LLMServiceError
LLMReportService holds real constructor state (base_url, api_key,
model, timeout_seconds) used across its methods, and encapsulates the
network call plus the thinking-block-stripping post-processing behind one
generate_report() method - a natural fit for a class.
LLMServiceError is a small custom exception type so callers can catch
"the LLM failed" distinctly from a generic httpx error.
src/nf_hotel_api/services/report.py - class ReportService
The orchestrator. Takes the other three services as constructor
dependencies and wires the raw-bookings -> clean -> stats -> LLM pipeline in
one generate() method. This is dependency injection in practice: each
collaborator is swappable, which is exactly how the tests substitute a fake
LLM service without touching the real cleaning/statistics logic.
API layer (FastAPI wiring) - deliberately function-based
src/nf_hotel_api/api/deps.py - functions: get_cleaning_service, get_stats_service, get_llm_service, get_report_service
FastAPI dependency-provider functions. FastAPI's Depends() system is
built around plain callables: it inspects a function's parameters and
return type to build the dependency graph and the OpenAPI schema. Each
function here does exactly one thing - construct and return a service
instance - and holds no state of its own, so wrapping it in a class would
add a layer of indirection with no benefit.
src/nf_hotel_api/api/v1/endpoints/report.py - functions: generate_report_from_file, generate_report_from_json
Route handlers. FastAPI's @router.post(...) decorators are applied to
functions; this is the idiomatic (and for FastAPI, effectively required)
shape for an endpoint. Each handler is stateless - it receives its
dependencies via Depends(...) and immediately delegates to the
class-based service layer to do the actual work.
src/nf_hotel_api/api/v1/router.py - no functions or classes, just module-level wiring
Purely declarative: creates one APIRouter and registers the report
router on it. There is no behavior here to encapsulate in either a
function or a class.
src/nf_hotel_api/main.py - module-level app + function health_check
Creates the FastAPI app instance and registers middleware/routers at import
time - this file is the composition root. health_check is a trivial,
stateless liveness probe; a single-method class here would add nothing
over a function.
Summary table
| File | Shape | Why |
|---|---|---|
domain/schemas.py |
Classes (Pydantic models) | Required by Pydantic/FastAPI for validation + OpenAPI schema |
core/config.py |
Class (Settings) + factory function |
BaseSettings requires a class; caching a constructor call only needs a function |
core/security.py |
Class | Reusable, testable, extensible FastAPI dependency object |
repositories/booking_repository.py |
Classes (ABC + 2 impls) | Interchangeable implementations of one contract - inheritance/polymorphism |
services/cleaning.py |
Class | Groups related private steps behind one public operation |
services/statistics.py |
Class | Consistency with sibling services; injectable into ReportService |
services/llm_report.py |
Classes | Holds constructor state (URL, key, model, timeout) used across methods |
services/report.py |
Class | Orchestrator with injected, swappable collaborators |
api/deps.py |
Functions | FastAPI Depends() expects plain callables; no state to hold |
api/v1/endpoints/report.py |
Functions | FastAPI route handlers must be functions; stateless delegation |
api/v1/router.py |
Module-level wiring | No behavior to encapsulate |
main.py |
Module-level app + 1 function | Composition root; trivial health check |