nigig-org/REVIEWS/COST ESTIMATE REVIEW.md
2026-07-26 19:38:26 +03:00

6.7 KiB
Raw Permalink Blame History

analyse if we have covered all the review below If I were taking over this project, I would freeze feature development for 12 weeks and do nothing but architecture work. The code is at the point where every new feature will either become much easier or much harder depending on what you do next.

This is the roadmap I'd follow.


Phase 1 — Establish a Real Domain (Highest Priority)

Goal

The application should revolve around an Estimate, not a screen.

Today the execution flow is roughly:

User

↓

CostEstimateScreen

↓

EstimateStore

↓

Estimate

I want:

User

↓

Command

↓

Estimate

↓

Domain Event

↓

UI

The UI should never decide how an estimate changes.


Create explicit commands

Instead of:

screen.set_region(...)
screen.recalculate(...)

everything becomes:

SetRegion

SetProperty

AddRoom

DeleteRoom

UpdateRoomDimensions

SetFloorCount

ImportEstimate

ExportEstimate

Every state change enters the system exactly one way.


Estimate becomes the application

Today Estimate still behaves like a data structure.

It should become the application.

It should own:

Estimate

Rooms

Selection

Totals

Floors

Property

Region

Validation

Calculations

Nothing outside Estimate should modify these.


Phase 2 — Remove UI Orchestration

This is the biggest architectural problem today.

Current pattern:

set property

↓

reload dropdown

↓

reload rooms

↓

update preview

↓

update totals

↓

emit action

That chain should disappear.


Instead:

SetProperty Command

↓

Estimate updates

↓

EstimateChanged event

↓

UI redraws

The screen should become stupid.

Its job should be:

Receive Event

↓

Refresh Widgets

That's it.


Phase 3 — Introduce Services

Estimate is already growing.

Don't let it become another monolith.

Split responsibilities.

RateService

lookup_rate()

lookup_region()

lookup_property()

RoomTemplateService

available_rooms()

default_dimensions()

room_generation()

CostService

calculate_room()

calculate_total()

floor_multiplier()

ExportService

PDF

CSV

JSON

Estimate coordinates them.

It doesn't implement them.


Phase 4 — Eliminate Manual Synchronisation

This is the smell I dislike most.

Current code has many functions like:

refresh_property()

refresh_header()

refresh_dropdown()

reload_rooms()

sync_preview()

Instead introduce a ViewModel.

Estimate

↓

EstimateViewModel

↓

Widgets

Widgets bind to data.

No widget should know another widget exists.


Phase 5 — Build a Proper State Machine

Instead of scattered booleans

loading

dirty

editing

calculating

make them explicit.

enum EstimateState {
    Loading,
    Ready,
    Editing,
    Saving,
    Exporting,
}

State machines eliminate entire categories of bugs.


Phase 6 — Introduce Immutable Snapshots

Today changes happen directly.

I'd rather have

Estimate

↓

clone

↓

modify

↓

replace

This immediately enables

  • Undo
  • Redo
  • Autosave
  • Time travel debugging
  • Change history

Phase 7 — Replace Nested Searches

Current:

Category

↓

Region

↓

Property

↓

Room

↓

Rate

every lookup.

After JSON loads build indices.

PropertyIndex

RateIndex

RoomIndex

RegionIndex

Everything becomes O(1).


Phase 8 — UI Decomposition

Current screen still owns too much.

Split into

EstimateHeader

EstimateSidebar

RoomEditor

TotalsPanel

PropertyPanel

RoomPanel

StatusBar

Each owns only its widgets.


Phase 9 — Stronger Types

I would remove lots of

String

usage.

Replace with

PropertyId

RegionId

RoomId

EstimateId

Compiler catches mistakes.


Phase 10 — Events Everywhere

Instead of

screen

↓

store

↓

screen

↓

preview

everything communicates through events.

Example:

RoomUpdated

↓

Estimate recalculates

↓

TotalsChanged

↓

Header redraw

↓

RoomUpdated

↓

Preview redraw

No direct widget calls.


Phase 11 — Testing

Current testing is okay.

I'd increase it dramatically.

Test

  • property switching
  • room generation
  • totals
  • floor multiplier
  • imports
  • exports
  • region changes
  • invalid JSON
  • invalid rooms
  • duplicate rooms
  • undo

UI tests are far less valuable than domain tests.


Phase 12 — Performance

Do this last.

Optimise

  • lookup indices
  • redraw batching
  • cached calculations

Ignore

  • clone
  • Vec
  • HashMap

They're not your bottleneck.


The Architecture I Would End Up With

                 UI
                  │
        EstimateViewModel
                  │
             Event Bus
                  │
        ┌─────────┴─────────┐
        │                   │
    Commands            Domain Events
        │                   │
        └─────────┬─────────┘
                  │
              Estimate
                  │
     ┌────────────┼─────────────┐
     │            │             │
RoomStore   CostService   RateService
     │            │             │
     └────────────┴─────────────┘
                  │
          Building Repository
                  │
                 JSON

What I'd do first (in order)

  1. Move all business rules into Estimate. Nothing outside it should calculate totals, update rooms, or apply property changes.
  2. Replace the screen's manual refresh chains with domain events. A state change should naturally produce an event that the UI responds to.
  3. Extract calculation and lookup logic into dedicated services. Keep Estimate as the coordinator rather than the implementation of every algorithm.
  4. Introduce lookup indices after loading configuration. This simplifies logic and prepares the application for larger datasets.
  5. Split the screen into smaller presentation components once the domain layer is stable.

One thing I would not do

I would not create more files just to reduce file size.

Splitting a 600-line file into six 100-line files doesn't improve architecture if the responsibilities stay the same.

Every extraction should answer one question:

Does this give a single component clear ownership of a single responsibility?

If the answer is no, it's just moving code around.

That's the principle I'd use to guide every refactor from here onward.