Do you remember your first day at CYF? You couldn’t find the building, maybe, and you had no idea how the day would go. What on earth is a day plan, or a backlog, you thought to yourself. Perhaps you got frustrated: why are all my changes from last week in my new PR? How?! It was incomprehensible. But you learned! You asked questions, you read the guides, and you built a mental map of the system.

You might have found your first code reviews challenging too. You worked on a project for days, just got it all working, and now someone is telling you to change it. Changing code you don’t understand very well feels risky.

Feeling a bit hesitant is fine. In fact, some caution is healthy. If code is working and it’s doing something important for the business, we don’t want to break it. But we also don’t want to be so fearful that we can’t fix it or write new features. We must balance caution with curiosity. We will approach legacy code with a structured, logical plan.

A good rule here is Chesterton’s Fence. This says that before we change something, we must explain why it’s like that in the first place.

In code, Chesterton’s Fence comes up a lot when we read code that looks complicated. It’s easy to think “This code could be simpler”. And maybe it could! There are a lot of reasons code is more complicated than it could be. Maybe it is complicated because:

• ✅ the person who wrote it didn’t know a better way. If so, we can simplify it.
• ✅ the simpler way was only introduced to the language after the code was written. If so, we can simplify it.
• 🚫 we want to support old versions of the language when the simpler way didn’t exist. If so, we can’t simplify it: we would break something important!
• 🚫 some important edge-case we hadn’t considered. If so, we need to understand that edge-case before we can change it, or we’ll break it.

Understanding why is crucial here. Tests can help us to understand. If we simplify the code and a test for a particular edge-case breaks, we found out why the code was more complicated! Comments can help too. A comment saying “We don’t do the simpler thing because it doesn’t handle undefined properly” tells us why the code is more complicated. But sometimes legacy code doesn’t have useful tests or comments.

Patterns are reusable solutions to common problems.

In the PurpleForest application, there are identifiable patterns. These are rules that the original developers followed when they built the application. By understanding these patterns, you can understand how the application works and how to extend it.

Code Conventions

We can also see regularities in the codebase called conventions. You can derive useful information from these conventions. Write down your answers to the following questions:

Component Creation Pattern

Let’s identify a convention in the Purple Forest application. Read any “createComponent” function in the Purple Forest codebase. In your notebook, write down the general steps this function takes to create a component.

// function name starts with create
// then name of file
// function expects a template (id) and data as arguments {
// first, return if there's no data
// next, clone template to create a fragment
// then, populate the fragment with data
// return fragment
//}

Check your specification against another “createComponent” function. Does the pattern hold?

Why Patterns Matter

Patterns help you:

• Predict how other parts of the system work
• Guide your implementation of new features
• Spot potential problems where patterns are broken

Understanding patterns allows you to chunk🧶🧶 chunkChunking is a way to group information together so it’s easier to remember and understand. information, making complex codebases easier to comprehend.

Enforcing patterns

Sometimes these patterns are enforced by the shape of the code, e.g. renderEach ensures that if we’re rendering multiple pieces of data we will always have the data, a container, a template name, and a create* function.

Other times these patterns are just implemented by writing code in similar styles. This is less reliable - it’s easy for someone to forget a step. Particularly this is harder to keep consistent over time - if you change the pattern, you need to make sure you update it everywhere.

Relying on code constructs (like functions) to enforce our use of patterns makes them more likely to be consistently followed.

It’s easier to fix a bug if you can reproduce it. Reproducing a bug allows you to see what’s happening, inspect what assumptions are incorrect, and verify that you have actually fixed it when you think you have.

Bugs without reproduction steps are often harder to debug and fix that ones with reproduction steps. Ideally reproduction steps tell you everything you need to do to reproduce the bug.

But it’s important to remember: reproduction steps are just an example. Our goal in debugging isn’t just to make the reproduction steps no longer show the bug. It’s to understand what the underlying problem was, and prevent that.

If, for instance, the reproduction steps mention a problem on one particular page, but that problem will also show up on other pages, it isn’t enough to just fix the one page given in the reproduction steps. We need to think about what caused the problem, where else it may occur, and how we can prevent the underlying problem, not just the example.

Tests are the best documentation for how a system should behave.

Describe the Behaviour, Capture your Understanding

Now we have an understanding of the system, the bug, and the intended behaviour, we need to write a test to capture our understanding. There are some Playwright tests in the codebase already, but no project has complete test coverage for absolutely every eventuality.

Launch the test runner (look in package.json to find the command). Find the file to add your test and describe the behaviour you expect.

📝 Activity: Write a test for the hashtag endpoint

// Given I am logged in
// When I navigate to /#/hashtag/do
// Then the number of fetch requests should be 1

test("should not make infinite hashtag endpoint requests", async ({ page }) => {
  // ===== ARRANGE
  const requests = [];
  page.on("request", (request) => {
    if (
      request.url().includes(":3000/hashtag/do") &&
      request.resourceType() === "fetch"
    ) {
      requests.push(request);
    }
  });
  // ====== ACT
  // When I navigate to the hashtag
  await page.goto("/#/hashtag/do");
  // And I wait a reasonable time for any additional requests
  await page.waitForTimeout(200);

  // ====== ASSERT
  // Then the number of requests should be 1
  expect(requests.length).toEqual(1);
});

Your test should be failing because your system is making too many requests. This is a good thing! It means you have a clear goal for your next step.

When we fix our code, having written this test means someone won’t be able to break this in the future without noticing. (If they’d written the test in the first place, we would never have had to debug this!)

For Purple Forest, this means:

1. Following the component-based architecture
2. Using the single source of truth pattern for state
3. Extending the API service for data fetching and updates if necessary

In your backlog, you have some more features to add. Let’s do one simple feature extension together. Branch from main to feature/unfollow.

Add an “Unfollow” button to the Profile component. This button should remove the current user from the list of followers. The button should only appear if the current user is following the user.

test("allows unfollowing a user from their profile", async ({ page }) => {
  await signUp(page, "sample");
  await signUp(page, "AnotherUser");

  // Given a profile component AnotherUser
  // And I am logged in as sample
  await loginAsSample(page);
  await page.goto("/#/profile/AnotherUser");
  // And sample is following AS
  await page.click('[data-action="follow"]');

  // When I view the profile component for AnotherUser
  // Then I should see a button labeled "Unfollow"
  const unfollowButton = page.locator('[data-action="unfollow"]');
  await expect(unfollowButton).toBeVisible();

  // When I click the "Unfollow" button
  await unfollowButton.click();

  // Then I should no longer be following AnotherUser
  const followerCount = page.locator("[data-follower-count]");
  await expect(followerCount).toHaveText("0");
  // And the unfollow button is not visible
  await expect(unfollowButton).toBe("hidden");
});

@jwt_required()
def do_unfollow():
    type_check_error = verify_request_fields({"unfollow_username": str})
    if type_check_error is not None:
        return type_check_error

    current_user = get_current_user()

    follow_username = request.json["unfollow_username"]
    unfollow_user = get_user(follow_username)
    if unfollow_user is None:
        return make_response(
            (f"Cannot unfollow {unfollow_username} - user does not exist", 404)
        )

    unfollow(current_user, unfollow_user)
    return jsonify(
        {
            "success": True,
        }
    )

✍🏾 Document your feature in your PR message.

Once you have got the entire feature working end to end with tests, open a PR with your changes. In your PR message, write everything that you needed to know to build this feature. Be good to the reviewer, they’re a good friend of yours.

📅 Schedule a revision

Schedule a revision in your calendar for two weeks from today to come back and review your own PR. You might choose to refactor your feature in your review.