## Approach The core idea is **batching**. Each call to the underlying `FileWriter::write` is expensive because it hits the OS, so we accumulate small writes in an in-memory buffer and only forward to the underlying writer when the buffer reaches a threshold. This trades a tiny amount of memory and write latency for a large reduction in syscalls — the standard motivation behind `BufferedWriter` / `BufWriter` / stdio's `setvbuf`. Two things to get right: 1. **Composition over inheritance.** `BufferedFileWriter` *has-a* `FileWriter`; it is not a kind of `FileWriter`. Composition keeps the interfaces decoupled and is what the prompt asks for. 2. **The flush contract.** `flush()` must (a) push any buffered bytes to the underlying writer, then (b) call the underlying `flush()` so the OS actually persists them. The capacity trigger inside `write()` does **only (a)** — it forwards to the underlying writer but does **not** call the underlying `flush()`, because the goal is to reduce syscalls, not to fsync on every fill. --- ## Part 1: `BufferedFileWriter` ```cpp #include <string> class BufferedFileWriter { public: BufferedFileWriter(FileWriter& underlying, size_t buffer_capacity) : underlying_(underlying), capacity_(buffer_capacity) { // A capacity of 0 would mean "flush on every write", which defeats the // purpose of buffering. That's degenerate but not unsafe; clamp to 1 so // the threshold check below is always meaningful. if (capacity_ == 0) capacity_ = 1; buffer_.reserve(capacity_); // avoid repeated reallocation } // Append data to the in-memory buffer. Forward to the underlying // writer only when the buffer reaches capacity. void write(const std::string& data) { buffer_.append(data); if (buffer_.size() >= capacity_) { flushBuffer(); // forward bytes, do NOT fsync } } // Push everything buffered, then durably flush the underlying writer. void flush() { flushBuffer(); underlying_.flush(); } private: // Forward the buffered bytes to the underlying writer and clear the // buffer. Safe to call when the buffer is empty (no-op). void flushBuffer() { if (!buffer_.empty()) { underlying_.write(buffer_); buffer_.clear(); // keeps reserved capacity in std::string } } FileWriter& underlying_; size_t capacity_; std::string buffer_; }; ``` ### Design notes / edge cases - **`>=` not `>`.** The prompt is explicit: flush when the post-append size is *greater than or equal to* capacity. Using `>` would let the buffer sit exactly at capacity — functionally fine but off-spec. - **Empty buffer is a no-op.** `flushBuffer()` guards on `!buffer_.empty()`, so calling `flush()` repeatedly with nothing buffered never forwards an empty string and never errors. This satisfies "multiple `flush()` calls with an empty buffer are safe." - **`clear()` retains capacity.** `std::string::clear()` does not release the reserved storage, so steady-state writes don't keep reallocating. - **The `capacity == 0` clamp.** `write()` is a single append plus one `if` — there is no loop, so a `0` capacity can't spin; it would just forward on every call. The clamp only keeps "buffered" behavior sane. - **Large writes.** The prompt says inputs are small relative to capacity. Even an oversized single chunk is handled correctly: it lands in the buffer and is immediately forwarded because `size() >= capacity_`. A more advanced variant would bypass the buffer for a chunk larger than capacity (see Follow-ups), but that is beyond the stated assumptions. - **Durability lives in one place.** We rely on `FileWriter::flush()` for persistence; `BufferedFileWriter::flush()` is the only path that guarantees bytes are on disk. **Complexity.** `write` is amortized $O(|data|)$ — each byte is copied into the buffer once and forwarded once. Over $N$ total bytes with capacity $C$, the number of underlying `write` calls is $O(N/C)$ instead of one per `write`, which is the whole point. Space is $O(C)$. --- ## Unit tests To assert *when* data is forwarded (not just the final bytes), the tests need a `FileWriter` that records each call. There is a subtlety that is easy to get wrong: the given `FileWriter::write`/`flush` are **non-virtual**, and `BufferedFileWriter` holds a `FileWriter&` and calls `underlying_.write(...)`, which dispatches **statically** to `FileWriter::write`. So a test double written as `class MockFileWriter : public FileWriter { void write(...) {...} }` would **not** intercept anything — the override is never reached, and (since the real `FileWriter::write` has no body in this exercise) the program would not even link. Subclassing only works if you are allowed to make `FileWriter` virtual, which the prompt does not grant. The clean way to get an inspectable double **without modifying the given interface** is to template `BufferedFileWriter` on the writer type. The logic is identical — it just no longer hard-codes `FileWriter` — and production code still uses it as `BufferedFileWriter<FileWriter>`. The test double is then any duck-typed type exposing `write`/`flush`; no inheritance, no virtuals, static dispatch lands on the recorder. ```cpp #include <string> // Same logic as Part 1, parameterized on the writer so it works with the // real (non-virtual) FileWriter AND with a recording test double. template <class Writer> class BufferedFileWriterT { public: BufferedFileWriterT(Writer& underlying, size_t buffer_capacity) : underlying_(underlying), capacity_(buffer_capacity) { if (capacity_ == 0) capacity_ = 1; buffer_.reserve(capacity_); } void write(const std::string& data) { buffer_.append(data); if (buffer_.size() >= capacity_) flushBuffer(); } void flush() { flushBuffer(); underlying_.flush(); } private: void flushBuffer() { if (!buffer_.empty()) { underlying_.write(buffer_); buffer_.clear(); } } Writer& underlying_; size_t capacity_; std::string buffer_; }; // Production alias: BufferedFileWriterT<FileWriter> == the Part 1 class. using BufferedFileWriter = BufferedFileWriterT<FileWriter>; ``` ```cpp #include <cassert> #include <string> #include <vector> // Standalone recorder — NOT derived from FileWriter, so the non-virtual // interface is irrelevant. It just exposes the same two methods by name. struct RecordingWriter { std::vector<std::string> writes; // each forwarded chunk, in order int flush_count = 0; void write(const std::string& data) { writes.push_back(data); } void flush() { ++flush_count; } std::string concatenated() const { std::string s; for (const auto& w : writes) s += w; return s; } }; ``` > Alternative if you must keep the concrete `FileWriter&` signature (no templates): wrap a real `FileWriter` whose implementation appends to an in-memory `std::string` sink instead of disk, and inspect that sink — same end-state coverage, though you lose the per-call `writes`/`flush_count` granularity unless the sink records call boundaries. Making `FileWriter` virtual would also work, but that changes the given interface, so I would only do it with the interviewer's OK. ```cpp // 1. Data smaller than capacity stays buffered until flush(). void test_small_write_then_flush() { RecordingWriter rec; BufferedFileWriterT<RecordingWriter> w(rec, /*capacity=*/100); w.write("hello"); assert(rec.writes.empty()); // nothing forwarded yet assert(rec.flush_count == 0); w.flush(); assert(rec.concatenated() == "hello"); assert(rec.writes.size() == 1); // one batched forward assert(rec.flush_count == 1); // underlying flush called } // 2. Chunks that together exceed capacity flush at the right time. void test_chunks_exceed_capacity() { RecordingWriter rec; BufferedFileWriterT<RecordingWriter> w(rec, /*capacity=*/8); w.write("abcd"); // size 4 < 8 -> buffered assert(rec.writes.empty()); w.write("efgh"); // size 8 >= 8 -> forwarded assert(rec.writes.size() == 1); assert(rec.writes[0] == "abcdefgh"); assert(rec.flush_count == 0); // capacity flush does NOT fsync w.write("ij"); // back in buffer, size 2 w.flush(); assert(rec.concatenated() == "abcdefghij"); assert(rec.flush_count == 1); } // 3. flush() on an empty buffer is safe and idempotent. void test_repeated_empty_flush() { RecordingWriter rec; BufferedFileWriterT<RecordingWriter> w(rec, /*capacity=*/16); w.flush(); w.flush(); assert(rec.writes.empty()); // never forwarded an empty chunk assert(rec.flush_count == 2); // underlying flush still invoked } // 4. (Good extra) No data is lost across the exact boundary. void test_exact_boundary() { RecordingWriter rec; BufferedFileWriterT<RecordingWriter> w(rec, /*capacity=*/3); w.write("xyz"); // size 3 >= 3 -> immediate forward assert(rec.writes.size() == 1 && rec.writes[0] == "xyz"); w.flush(); // nothing left to forward assert(rec.concatenated() == "xyz"); } int main() { test_small_write_then_flush(); test_chunks_exceed_capacity(); test_repeated_empty_flush(); test_exact_boundary(); return 0; } ``` The key assertions are *behavioral*, not just final-state: test 1 proves data is withheld until `flush()`, and test 2 proves the capacity trigger forwards without an fsync. That distinction — provable only because the recorder actually intercepts each underlying call — is what the interviewer is checking. --- ## Part 2: Thread-safe `BufferedFileWriter` ### Step 1 — state the guarantee precisely With multiple threads calling `write()` and `flush()` concurrently, I provide **linearizability of the public operations**: every `write` and `flush` appears to take effect atomically at some point between its call and return, consistent with *some* serial order of all calls. Concretely: - The internal buffer is never read/written by two threads at once — no data races, no torn `std::string` state. - No bytes are lost or duplicated, and the bytes from a single `write(data)` call are forwarded to the underlying writer **contiguously**, in one piece (never interleaved with another thread's bytes). - After a `flush()` returns, **all writes that completed before that `flush()` call began** are durably on disk. What I do **not** promise: a global ordering across threads that matches wall-clock time (the OS scheduler decides which concurrent `write` "wins"), nor any ordering between two genuinely concurrent `write` calls. For a log/file writer that is the right contract — callers needing strict ordering must serialize at a higher level or use one writer per stream. ### Step 2 — mechanism The simplest correct design is **one mutex guarding all shared state** (the buffer). Both `write` and `flush` take the lock for their whole critical section. This makes every operation mutually exclusive, which trivially gives the serial-order guarantee. The trade-off is that the I/O happens under the lock, so a slow underlying `write` blocks other threads. ```text class ThreadSafeBufferedFileWriter: field underlying # FileWriter& field capacity # size_t field buffer = "" # shared, protected by mu field mu # mutex method write(data): lock(mu) # acquire exclusive access buffer.append(data) if buffer.size() >= capacity: if not buffer.empty(): underlying.write(buffer) # forward under lock -> atomic chunk buffer.clear() unlock(mu) method flush(): lock(mu) if not buffer.empty(): underlying.write(buffer) buffer.clear() underlying.flush() # durability under lock unlock(mu) ``` In real C++, use RAII (`std::lock_guard<std::mutex>`) instead of manual `lock/unlock` so the lock releases even if `underlying.write` throws. ### Step 3 — why this is race-free Only `mu` ever touches `buffer`, and both methods hold `mu` for the entire append-and-maybe-forward sequence. So: - Two `write`s cannot interleave their `append`s → no torn buffer. - A `write`'s append and a `flush`'s drain cannot overlap → no lost or double-forwarded bytes. - Because `underlying.write(buffer)` runs while the lock is held, each forwarded chunk is exactly one thread's accumulated buffer, never a splice of two threads' data. ### Reducing time under lock (the senior-level refinement) Holding the mutex across the (slow) syscall serializes all I/O. A common improvement is to **swap the buffer out under the lock, then do the I/O outside it**: ```text method write(data): local to_send = "" lock(mu) buffer.append(data) if buffer.size() >= capacity: to_send = move(buffer) # hand the full buffer to this thread buffer = "" # fresh empty buffer for others unlock(mu) if to_send not empty: underlying.write(to_send) # I/O outside the lock ``` This shrinks the critical section to a cheap append + pointer swap. **But it introduces an ordering hazard:** two threads can both swap, leave the lock, and then race in `underlying.write`, so chunk *B* (swapped later) could hit disk before chunk *A*. If the file's byte order must reflect flush order, this is wrong. The fix is a **second "I/O mutex"** that serializes only the underlying writes, acquired in swap order — or simply keep the single-lock design and accept serialized I/O. In an interview I would present the single-lock version as the correct default and offer the swap optimization *with its ordering caveat called out*, because volunteering the caveat is what distinguishes a senior answer. ### Other points worth raising - **`flush()` durability scope.** My guarantee covers writes that *completed before* `flush` was called. A `write` racing with `flush` may or may not be included — inherent to concurrency, and should be stated, not hidden. - **No condition variable needed (yet).** Condition variables matter when a thread must *wait* for a state change (a background flush thread sleeping until the buffer is non-empty, or a bounded buffer applying backpressure). The straightforward synchronous design has no waiting, so a plain mutex suffices. - **Reentrancy.** `std::mutex` is non-recursive, so `write` must not call a public method that re-locks. The code above takes the lock once per call, so there is no self-deadlock. - **Atomics are not enough.** Making `buffer` "atomic" does not work — append-and-conditionally-forward is a multi-step read-modify-write across a whole string; only a lock (or a lock-free queue redesign) makes it atomic. --- ## Addressing the follow-up questions **Background / time-based flush thread.** Add a dedicated flusher: a condition variable `cv` plus a `running` flag. `write` notifies `cv` after appending; the flusher loops `cv.wait(lock, [&]{ return !buffer.empty() || !running || deadline_passed; })`, drains the buffer, and releases the lock for the syscall (using the swap trick + an I/O-ordering lock so background and foreground flushes do not reorder bytes). For time-based flushing, `wait_for(lock, flush_interval)` wakes on a timer even when no write arrives. Clean shutdown: set `running = false` under the lock, `cv.notify_all()`, then `join()` the thread, and do a final `flush()` so no buffered bytes are dropped. **A single `write` larger than `buffer_capacity`.** The Part 1 code still behaves correctly — the oversized chunk is appended, `size() >= capacity_` is true, and it is forwarded immediately in one underlying `write`. The only inefficiency is the extra copy into the buffer. To special-case it: if `buffer_` is empty and `data.size() >= capacity_`, forward `data` straight to `underlying_.write(data)` and skip the buffer entirely; otherwise if appending would overflow, first flush the existing buffer, then forward the large chunk directly. This avoids buffering data that will be flushed on the very next line. **Crash window and durability modes.** Between a capacity-triggered forward and the next `flush()`, bytes live in the OS page cache (handed off via `underlying_.write`) but are not necessarily on stable storage; un-forwarded bytes still in our in-memory buffer are lost entirely on a crash. `BufferedFileWriter::flush()` plus `FileWriter::flush()` bounds that window to "everything written before the last `flush()`." A stricter mode would call `flush()` on the underlying writer after every capacity-triggered forward (fsync-per-batch), or expose a "durable write" path that bypasses buffering and flushes immediately — trading throughput for a smaller loss window. `O_DIRECT`/`fsync`-per-write would be the extreme end of that spectrum. **Backpressure.** To stop a fast producer from growing memory unbounded while the underlying writer is slow, cap the number of in-flight/pending buffers (or total buffered bytes) and make `write` **block** when the cap is reached. This needs a condition variable: `write` waits on `cv_not_full` while pending bytes exceed the limit; the flusher signals `cv_not_full` after draining. The deadlock to avoid is calling the blocking `write` from inside the flush/drain path (or holding the buffer mutex while waiting on the syscall) — the wait must release the mutex (`cv.wait` does this) and the producer and consumer must never each hold the resource the other needs.