Review of Claude Code-generated code for a bookshop, March 2026

README.md

Context

This is code generated as part of our course on agentic coding. The code below was written by Claude Code using its default model in late Feb 2026.

Learning Objective

The purpose of this assigment was to help students realize that, in the absence of explicit prompting to this effect, Claude Code (in its current state) is highly unlikely to generate code with a clean separation of concerns. The specific concern we wanted to focus on was the factoring-out of business rules. This is to set up students to learn about business rules, author them separately, and then figure out how to incorporate them into the program: e.g., using something like the RETE algorithm.

The hope is that once students learn this concept, they will recognize it in future tasks, and think to prompt for it to be done more deliberately. Since Claude doesn't do it automatically, it's something they need to be taught about. So we wanted to set up for thinking about it.

In the process of doing that we found several other issues to highlight in the code, all of which are useful for relative novice students (the target audience) to learn to think about. Hence this writeup.

Task

Our working example here is an online bookstore. It displays books, which have genres. Users can add books to a shopping cart.

Books are specified externally in a CSV file.

The bookstore offers certain discounts:

• Summer Coding Sale: 20% off programming books
• 30% off when buying 3+ books
• Free shipping over 50

Claude's job was to generate a front-end and back-end for this. It was also asked to generate a test suite.

Code

We intentionally asked Claude to generate TypeScript code. The types provide a useful medium for auditing and for communication (in my experience, sometimes even for expressing user interface decisions). The types will prove to be especially useful here.

Activity

In class, we reviewed the back-end code. This is a rough sense of what I went over in my review. This gist has four files:

• this README
• the test suite (tests.ts)
• the store's back-end (store.ts)
• my review (review.md)

I strongly encourage you to first read and form your own impressions of the testing and back-end code, before reading my review.

I should add that this was not a "deep" review. I had ~25 minutes to do this before catching a flight, so there may well be other lurking issues. But I also think this is representative of the time that people will spend (at most). Also, you may not agree on some of my design choices!

Disclaimers

• Humans would also make similar decisions.
• These certainly seem fixable. Indeed, the course will be working on fixing them.
• This isn't meant to be deep criticism of agentic coding, but it is the kind of thing that demands caution.
• This course is designed for relatively novice programmers.

review.md

Let's start with tests. I'm going to only superficially look at the tests file.

Let's note that it's quite long and elaborate, but it seems to test a lot of boundary conditions but not a lot of testing for the bulk of the logic.

Also, I would have really liked to see property-based tests. A store has several algebraic properties: e.g., if you add something to a store and then take it out, you should get back to the original store. This sort of thing seems very testable.

We'll get back to the test file later. But let's proceed with the back-end code.

I like to start by auditing types:

Product looks pretty good. If you're unsettled by some things, hold on: we'll come back to them later. But this is a good first cut at a bookstore product entry. Though it is a bit unsettling that it says Product and author; usually books have authors, while products have, maybe, creators? So the naming seems inconsistent.

CartEntry and PricingInfo look fine as types. But looking through the code, PricingInfo is hardly ever used. This should be a bit suspicious. Now, it's sometimes okay: e.g., a type may be used in the interface but not much in the rest of the implementation. But this is a "smell test" to audit for.

LineItem has a funny whiff. Why are there so many things in it? Also, just from a types perspective, it seems to duplicate fields in CartEntry. Furthermore, those aren't unrelated. So should these really be two different types?

CartSummary looks fine. But it's interesting that a cart summary is a collection of LineItems, not CartEntrys, which compounds the previous comment.

Okay, so much for the types (for now). Now let's start looking at the code:

Fiona Hopkins pointed out that comments describing functions should be written in JSDoc format for editor integration.

parseProductsCsv looks fine: take a CSV file, use a standard CSV parser. Oh wait, that's not what it's doing at all. It seems to have its own CSV parser. Surely there are standard CSV parsers available? There are all kinds of issues w/ spinning your own (can you list them?). Also, there's a "Semicolon-Separated-Values" parser for genres. It's splitting by the character ;, which means spaces could become an issue… (The current CSV file happens to contain no spaces, but surely that will not persist. Indeed, it's hard to read the genres due to the lack of spaces.)

filterByTitle and filterByAuthor look very similar. A key thing is that they both encode a certain kind of data-normalization (ignore case). But this is now duplicated, which means if it's not consistently duplicated, different parts may be inconsistent. It would be better to separate what normalization you want and state a policy for each field, and do this very deliberately. Note also that this is missing various other things you might want, like ignoring leading or trailing spaces.

As a naming matter, I don't like the choice of filter…. To me, "filtering" is a purely mechanical activity. In contrast, these are functions that are ostensibly written to support a search box — hence the accommodation of case — which means it would be better to call them search….

addToCart is a functional update, not an imperative mutation. It would perhaps be better to therefore call it updateCart.

The code of addToCart is troubling. It starts with a conditional that is (hopefully) Boolean-valued. It makes sense to have this branch: if it's already in the cart you want to increment the count, else add it. But note the critical keyword missing: else. Instead, it follows this weird pattern of having a return + fall-through behavior. This is a bad application of a good idea. The good idea, often called "guarded clauses" or "early returns", is when you want to get the exceptional cases out of the way and have the function body just be the happy-path. This is a good thing! I've even done research to support this! But this is not such a case: there are two symmetric cases, neither of higher priority. I have to follow control-flow paths for no good reason. It violates a basic norm of structured programming.

Next, changeQuantity:

• First of all, the delta should not be any arbitrary number. Now the UI might only send it particular numbers, but if you ever expose this as an API, you no longer have that guarantee. There should be a guard to check this. (There should be several guards for many things, but this especially, because there is no other entity that will check this: you'll get silent bad-behavior.)
• In particular, delta should never let the total number fall below 0. If that happens, something has gone horribly wrong. Maybe there's a bug in the UI! This should be checked, not quietly glossed over.
• Next is the really weird filter. That suggests that it's aware that the value could drop below 0? Aha, so there must be a test case for that! Right? (Is there?)
• But also, it's not clear this is exactly the logic you want! Some stores let users keep 0-count items to induce them to add them back. Also, you may discover that there's a discount you become eligible for, and that may even make the total price lower. If you remove all the 0-elements, those become much harder to find. So really, this is a business and UI decision that has quietly been made by the code.

Next, getLineItemPricing:

• Again, we're doing this premature-return thing in a place where it doesn't make sense. There are essentially three parallel situations. But it's hard to tell what they are: to understand when the third return happens, you have to read and understand everything before it and correctly negate logic.
• Next, we've hard-coded all the policies! There are so many problems with this: a huge conflation of what should be separated concerns. (The separable concern here even has its own name: Business Rules. Handling this is our next major step.
• Third, there doesn't seem to be anything checking for when the policies should be in place. The summer sale seems to be on always?
• Fourth: Is the logic correct? Are you sure? Certain? (And if you ask: "What does correctness even mean?"…yeah!)
• Oh, there's one more thing…which we'll come back to. (See if you can spot it.)

One useful exercise is to step back and

• ask yourself how many different categories of tests there should be, and
• check whether they are, indeed, all fully represented in the test suite

There's more stuff like this lower down in the file, like another hard-coded policy. But now I want to return to the top of the file to bring up two more things.

First: all the prices are recorded as number. That's fine, right?

Well akshually: these are TypeScript number, which are JavaScript numbers, which are all just floating points.

This means, for instance, 80% of 49.99 should be 39.992, but it's actually 39.992000000000004 (which is not 39.992).

Ah, but did you notice the toFixeds all over the code? Those are ostensibly here to prevent horrible numbers like the above appearing in the output. But do they do the right thing?

Consider 70% of 19.95. Its true value is 13.965. As a float, that comes to 13.964999999999998, which with rounding is indeed 13.965. But (19.95 * 0.7).toFixed(2) produces the string '13.96'.

In short: you should never represent money as floating point numbers. Use a decimal library or better.

Second: genres are represented as strings. Is that okay?

There is always a trade-off between flexibility and correctness. But this is exactly the kind thing where you can expect to see many data-entry errors, problems when importing data from other sources, etc. Furthermore, recall that one of the discounts is attached to a genre, so it's especially important to get it right!

This is an interesting issue in TypeScript. We can use the language's enums to capture this. We can even use the strings directly as part of a datatype. Here's an example in the TypeScript playground that illustrates this at work.

Okay, that's enough. You might agree with everything above; you may also observe other things.

store.js

// EZShop™ — Product Catalog & Pricing Engine (Books Edition)

interface Product {
  id: number;
  name: string;
  author: string;
  genres: string[];
  price: number;
}

interface CartEntry {
  productId: number;
  quantity: number;
}

interface PricingInfo {
  salePrice: number;
  promotion: string | null;
}

interface LineItem {
  product: Product;
  quantity: number;
  unitPrice: number;
  lineTotal: number;
  promotion: string | null;
}

interface CartSummary {
  lineItems: LineItem[];
  subtotal: number;
  shipping: number;
  shippingPromo: string | null;
  total: number;
}

// Parses a CSV string (with header row) into an array of Products.
// Genres are semicolon-separated within the CSV field.
function parseProductsCsv(csv: string): Product[] {
  return csv.trim().split("\n").slice(1).map(line => {
    const [id, name, author, genres, price] = line.split(",");
    return { id: +id, name, author, genres: genres.split(";"), price: +price };
  });
}

// Returns products whose title matches the query string.
function filterByTitle(products: Product[], query: string): Product[] {
  const q = query.toLowerCase();
  return products.filter(p => p.name.toLowerCase().includes(q));
}

// Returns products whose author matches the query string.
function filterByAuthor(products: Product[], query: string): Product[] {
  const q = query.toLowerCase();
  return products.filter(p => p.author.toLowerCase().includes(q));
}

// Sorts products by name alphabetically.
function sortByTitle(products: Product[]): Product[] {
  return [...products].sort((a, b) => a.name.localeCompare(b.name));
}

// Sorts products by price, lowest first.
function sortByPrice(products: Product[]): Product[] {
  return [...products].sort((a, b) => a.price - b.price);
}

// Adds one unit of a product to the cart, returning a new cart.
function addToCart(cart: CartEntry[], productId: number): CartEntry[] {
  const existing = cart.find(item => item.productId === productId);
  if (existing) {
    return cart.map(item =>
      item.productId === productId
        ? { ...item, quantity: item.quantity + 1 }
        : item
    );
  }
  return [...cart, { productId, quantity: 1 }];
}

// Changes the quantity of a product in the cart by delta, returning a new cart.
// Removes the entry if quantity drops to zero or below.
function changeQuantity(cart: CartEntry[], productId: number, delta: number): CartEntry[] {
  return cart
    .map(item =>
      item.productId === productId
        ? { ...item, quantity: item.quantity + delta }
        : item
    )
    .filter(item => item.quantity > 0);
}

// Returns the effective sale price and active promotion for a product.
// Needs the full cart because the bulk discount depends on total quantity.
function getLineItemPricing(product: Product, cart: CartEntry[]): PricingInfo {
  const totalItems = cart.reduce((sum, item) => sum + item.quantity, 0);

  // Summer Coding Sale — 20% off programming books, 30% off when buying 3+
  if (product.genres.includes("programming")) {
    if (totalItems >= 3) {
      return {
        salePrice: +(product.price * 0.70).toFixed(2),
        promotion: "Summer Coding Sale (30% off 3+)",
      };
    }
    return {
      salePrice: +(product.price * 0.80).toFixed(2),
      promotion: "Summer Coding Sale (20% off)",
    };
  }

  return { salePrice: product.price, promotion: null };
}

// Computes full pricing for a cart: resolves each entry to a line item
// with promotion-aware pricing, then derives subtotal, shipping, and total.
function calculateCart(cart: CartEntry[], products: Product[]): CartSummary {
  const lineItems: LineItem[] = cart.map(({ productId, quantity }) => {
    const product = products.find(p => p.id === productId)!;
    const { salePrice, promotion } = getLineItemPricing(product, cart);
    return {
      product,
      quantity,
      unitPrice: salePrice,
      lineTotal: +(salePrice * quantity).toFixed(2),
      promotion,
    };
  });

  const subtotal = +lineItems.reduce((sum, li) => sum + li.lineTotal, 0).toFixed(2);

  // Free shipping on orders over $50
  const shipping = subtotal >= 50 ? 0 : 5.99;
  const shippingPromo = subtotal >= 50 ? "Free shipping on orders over $50!" : null;

  return {
    lineItems,
    subtotal,
    shipping,
    shippingPromo,
    total: +(subtotal + shipping).toFixed(2),
  };
}

tests.ts

// EZShop™ Books — Test Suite
// Loaded after store.js; run via tests.html in the browser.

let _passed = 0;
let _failed = 0;
const _results: string[] = [];

function assert(condition: boolean, name: string) {
  if (condition) {
    _passed++;
    _results.push(`✓ ${name}`);
  } else {
    _failed++;
    _results.push(`✗ ${name}`);
  }
}

function assertClose(actual: number, expected: number, name: string) {
  assert(Math.abs(actual - expected) < 0.01, `${name} (got ${actual}, expected ${expected})`);
}

// ---- Test data ----

const TEST_PRODUCTS: Product[] = [
  { id: 1, name: "Book A", author: "Alice Smith", genres: ["programming", "software-engineering"], price: 40.00 },
  { id: 2, name: "Book B", author: "Bob Jones", genres: ["programming"], price: 20.00 },
  { id: 3, name: "Code C", author: "Alice Jones", genres: ["programming", "computer-science"], price: 50.00 },
  { id: 4, name: "Algorithms", author: "Carol White", genres: ["computer-science"], price: 60.00 },
  { id: 5, name: "Managing Teams", author: "Dan Green", genres: ["management"], price: 25.00 },
];

// ---- parseProductsCsv ----

function testParseCsv() {
  const csv = "id,name,author,genres,price\n1,Widget,Jane Doe,programming;cs,19.99\n2,Novel,John Smith,management,12.50\n";
  const products = parseProductsCsv(csv);

  assert(products.length === 2, "parseProductsCsv: correct row count");
  assert(products[0].name === "Widget", "parseProductsCsv: name parsed");
  assert(products[0].author === "Jane Doe", "parseProductsCsv: author parsed");
  assert(products[0].genres.length === 2, "parseProductsCsv: genres split");
  assert(products[0].genres[0] === "programming", "parseProductsCsv: first genre");
  assert(products[0].genres[1] === "cs", "parseProductsCsv: second genre");
  assert(products[1].genres.length === 1, "parseProductsCsv: single genre");
}

// ---- filterByTitle ----

function testFilterByTitleMatch() {
  const result = filterByTitle(TEST_PRODUCTS, "book");
  assert(result.length === 2, "filterByTitle: finds 2 books");
}

function testFilterByTitleEmpty() {
  const result = filterByTitle(TEST_PRODUCTS, "");
  assert(result.length === TEST_PRODUCTS.length, "filterByTitle empty: returns all");
}

function testFilterByTitleNoMatch() {
  const result = filterByTitle(TEST_PRODUCTS, "xyz");
  assert(result.length === 0, "filterByTitle no match: returns empty");
}

// ---- filterByAuthor ----

function testFilterByAuthorMatch() {
  const result = filterByAuthor(TEST_PRODUCTS, "alice");
  assert(result.length === 2, "filterByAuthor: finds 2 by Alice");
}

function testFilterByAuthorEmpty() {
  const result = filterByAuthor(TEST_PRODUCTS, "");
  assert(result.length === TEST_PRODUCTS.length, "filterByAuthor empty: returns all");
}

function testFilterByAuthorNoMatch() {
  const result = filterByAuthor(TEST_PRODUCTS, "xyz");
  assert(result.length === 0, "filterByAuthor no match: returns empty");
}

// ---- sortByTitle ----

function testSortByTitle() {
  const result = sortByTitle(TEST_PRODUCTS);
  assert(result[0].name === "Algorithms", "sortByTitle: Algorithms first");
  assert(result[1].name === "Book A", "sortByTitle: Book A second");
}

function testSortByTitleImmutable() {
  const original = [TEST_PRODUCTS[2], TEST_PRODUCTS[0]];
  const sorted = sortByTitle(original);
  assert(original[0].id === 3, "sortByTitle: original unchanged");
  assert(sorted[0].id === 1, "sortByTitle: sorted copy reordered");
}

// ---- sortByPrice ----

function testSortByPrice() {
  const result = sortByPrice(TEST_PRODUCTS);
  assert(result[0].price === 20.00, "sortByPrice: cheapest first");
  assert(result[result.length - 1].price === 60.00, "sortByPrice: most expensive last");
}

function testSortByPriceImmutable() {
  const original = [TEST_PRODUCTS[0], TEST_PRODUCTS[1]];
  const sorted = sortByPrice(original);
  assert(original[0].id === 1, "sortByPrice: original unchanged");
  assert(sorted[0].id === 2, "sortByPrice: sorted copy reordered");
}

// ---- addToCart ----

function testAddToCartNew() {
  const cart = addToCart([], 1);
  assert(cart.length === 1, "addToCart new: one entry");
  assert(cart[0].productId === 1, "addToCart new: correct product");
  assert(cart[0].quantity === 1, "addToCart new: quantity 1");
}

function testAddToCartExisting() {
  const cart = addToCart([{ productId: 1, quantity: 1 }], 1);
  assert(cart.length === 1, "addToCart existing: still one entry");
  assert(cart[0].quantity === 2, "addToCart existing: quantity incremented");
}

function testAddToCartImmutable() {
  const original: CartEntry[] = [{ productId: 1, quantity: 1 }];
  const updated = addToCart(original, 1);
  assert(original[0].quantity === 1, "addToCart: original unchanged");
  assert(updated[0].quantity === 2, "addToCart: new cart updated");
}

// ---- changeQuantity ----

function testChangeQuantityUp() {
  const cart = changeQuantity([{ productId: 1, quantity: 1 }], 1, 1);
  assert(cart[0].quantity === 2, "changeQuantity +1: quantity increased");
}

function testChangeQuantityDown() {
  const cart = changeQuantity([{ productId: 1, quantity: 3 }], 1, -1);
  assert(cart[0].quantity === 2, "changeQuantity -1: quantity decreased");
}

function testChangeQuantityRemoves() {
  const cart = changeQuantity([{ productId: 1, quantity: 1 }], 1, -1);
  assert(cart.length === 0, "changeQuantity to 0: entry removed");
}

function testChangeQuantityImmutable() {
  const original: CartEntry[] = [{ productId: 1, quantity: 2 }];
  const updated = changeQuantity(original, 1, -1);
  assert(original[0].quantity === 2, "changeQuantity: original unchanged");
  assert(updated[0].quantity === 1, "changeQuantity: new cart updated");
}

// ---- getLineItemPricing: programming books ----

function testPricingProgrammingOneItem() {
  const cart: CartEntry[] = [{ productId: 1, quantity: 1 }];
  const result = getLineItemPricing(TEST_PRODUCTS[0], cart);
  assertClose(result.salePrice, 32.00, "programming 1 item: 20% off");
  assert(result.promotion !== null, "programming 1 item: has promotion");
}

function testPricingProgrammingThreeItems() {
  const cart: CartEntry[] = [{ productId: 1, quantity: 3 }];
  const result = getLineItemPricing(TEST_PRODUCTS[0], cart);
  assertClose(result.salePrice, 28.00, "programming 3 items: 30% off");
  assert(result.promotion!.includes("30%"), "programming 3 items: promotion says 30%");
}

// ---- getLineItemPricing: computer-science books (no special discount) ----

function testPricingCsNoDiscount() {
  const cart: CartEntry[] = [{ productId: 4, quantity: 1 }];
  const result = getLineItemPricing(TEST_PRODUCTS[3], cart);
  assertClose(result.salePrice, 60.00, "cs-only: full price");
  assert(result.promotion === null, "cs-only: no promotion");
}

// ---- getLineItemPricing: no discount genres ----

function testPricingManagementNoDiscount() {
  const cart: CartEntry[] = [{ productId: 5, quantity: 1 }];
  const result = getLineItemPricing(TEST_PRODUCTS[4], cart);
  assertClose(result.salePrice, 25.00, "management: full price");
  assert(result.promotion === null, "management: no promotion");
}

// ---- calculateCart ----

function testEmptyCart() {
  const result = calculateCart([], TEST_PRODUCTS);
  assert(result.lineItems.length === 0, "empty cart: no line items");
  assertClose(result.subtotal, 0, "empty cart: subtotal 0");
  assertClose(result.shipping, 5.99, "empty cart: shipping charged");
  assertClose(result.total, 5.99, "empty cart: total = shipping only");
}

function testCartMixedDiscounts() {
  const cart: CartEntry[] = [
    { productId: 1, quantity: 1 },  // programming: $40 -> $28 (30% off, 3+ items)
    { productId: 4, quantity: 1 },  // cs-only: $60 (full price)
    { productId: 5, quantity: 1 },  // management: $25 (full price)
  ];
  const result = calculateCart(cart, TEST_PRODUCTS);
  assert(result.lineItems.length === 3, "mixed cart: 3 line items");
  assertClose(result.lineItems[0].unitPrice, 28.00, "mixed cart: programming at 30% (3+ items)");
  assertClose(result.lineItems[1].unitPrice, 60.00, "mixed cart: cs full price");
  assertClose(result.lineItems[2].unitPrice, 25.00, "mixed cart: management full price");
}

function testShippingUnder50() {
  const cart: CartEntry[] = [{ productId: 5, quantity: 1 }]; // $25, no discount
  const result = calculateCart(cart, TEST_PRODUCTS);
  assertClose(result.shipping, 5.99, "under $50: shipping charged");
  assert(result.shippingPromo === null, "under $50: no shipping promo");
}

function testShippingOver50() {
  const cart: CartEntry[] = [{ productId: 4, quantity: 1 }]; // $60 -> $51
  const result = calculateCart(cart, TEST_PRODUCTS);
  assertClose(result.shipping, 0, "over $50: free shipping");
  assert(result.shippingPromo !== null, "over $50: shipping promo present");
}

function testTotalEqualsSubtotalPlusShipping() {
  const cart: CartEntry[] = [{ productId: 5, quantity: 1 }];
  const result = calculateCart(cart, TEST_PRODUCTS);
  assertClose(result.total, result.subtotal + result.shipping, "total = subtotal + shipping");
}

// ---- Run all tests ----

function runAllTests() {
  testParseCsv();
  testFilterByTitleMatch();
  testFilterByTitleEmpty();
  testFilterByTitleNoMatch();
  testFilterByAuthorMatch();
  testFilterByAuthorEmpty();
  testFilterByAuthorNoMatch();
  testSortByTitle();
  testSortByTitleImmutable();
  testSortByPrice();
  testSortByPriceImmutable();
  testAddToCartNew();
  testAddToCartExisting();
  testAddToCartImmutable();
  testChangeQuantityUp();
  testChangeQuantityDown();
  testChangeQuantityRemoves();
  testChangeQuantityImmutable();
  testPricingProgrammingOneItem();
  testPricingProgrammingThreeItems();
  testPricingCsNoDiscount();
  testPricingManagementNoDiscount();
  testEmptyCart();
  testCartMixedDiscounts();
  testShippingUnder50();
  testShippingOver50();
  testTotalEqualsSubtotalPlusShipping();
}

Do you know if this was Opus or Sonnet?