James Shore: Challenge #9: Refactoring

Challenge #9: Refactoring

In the next challenge, you’ll flesh out the remaining edge cases. At present, though, errors are cumbersome to test and implement, because of the big exception message. In this challenge, you’ll refactor the code to make future edge cases easier to handle.

Instructions

1. Refactor the tests:

Extract an assertFailureAsync() function that runs transformAsync() and asserts that the desired error occurred.
Make all its parameters optional, with valid defaults.

2. Refactor the production code:

Extract a throwError() function that throws an error with helpful details about the request that failed.

Remember to commit your changes when you’re done.

API Documentation

No new methods.

JavaScript Primers

No new concepts.

Hints

Refactor the "fails gracefully when status code has unexpected value" test:

Factor out the test-specific variables so the test contains the generic code you want to extract.

Your editor might have an automatic “Introduce Variable” refactoring that does this for you.

it("fails gracefully when status code has unexpected value", async () => {
	const port = 9999;
	const rot13ServiceStatus = 400;
	const rot13ServiceHeaders = VALID_ROT13_HEADERS;
	const rot13ServiceBody = VALID_ROT13_BODY;
	const message = "Unexpected status from ROT-13 service";

	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
});

Refactor the newly-generic code into a separate function named assertFailureAsync(). Don’t forget the async and await keywords.

Your editor might have an automatic “Extract Method” or “Extract Function” refactoring that does this for you.

it("fails gracefully when status code has unexpected value", async () => {
	const port = 9999;
	const rot13ServiceStatus = 400;
	const rot13ServiceHeaders = VALID_ROT13_HEADERS;
	const rot13ServiceBody = VALID_ROT13_BODY;
	const message = "Unexpected status from ROT-13 service";

	await assertFailureAsync(port, rot13ServiceStatus, rot13ServiceHeaders, rot13ServiceBody, message);
});

// ...

async function assertFailureAsync(port, rot13ServiceStatus, rot13ServiceHeaders, rot13ServiceBody, message) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

TypeScript requires type declarations on the extracted function:

async function assertFailureAsync(
	port: number,
	rot13ServiceStatus: number,
	rot13ServiceHeaders: HttpHeaders,
	rot13ServiceBody: string,
	message: string,
) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

Make assertFailureAsync()’s parameters optional.

Use object destructuring with default values.

it("fails gracefully when status code has unexpected value", async () => {
	const port = 9999;
	const rot13ServiceStatus = 400;
	const rot13ServiceHeaders = VALID_ROT13_HEADERS;
	const rot13ServiceBody = VALID_ROT13_BODY;
	const message = "Unexpected status from ROT-13 service";

	await assertFailureAsync({ port, rot13ServiceStatus, rot13ServiceHeaders, rot13ServiceBody, message });
});

// ...

async function assertFailureAsync({
	port = 42,
	rot13ServiceStatus = VALID_ROT13_STATUS,
	rot13ServiceHeaders = VALID_ROT13_HEADERS,
	rot13ServiceBody = VALID_ROT13_BODY,
	message,
}) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

TypeScript’s type declarations move:

async function assertFailureAsync({
	port = 42,
	rot13ServiceStatus = VALID_ROT13_STATUS,
	rot13ServiceHeaders = VALID_ROT13_HEADERS,
	rot13ServiceBody = VALID_ROT13_BODY,
	message,
}: {
	port?: number,
	rot13ServiceStatus?: number,
	rot13ServiceHeaders?: HttpHeaders,
	rot13ServiceBody?: string,
	message?: string,
}) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

Clean up.

Modify the test so it only specifies the information that’s relevant to the test.

For the status code test, only the status code and expected error message matter.

it("fails gracefully when status code has unexpected value", async () => {
	await assertFailureAsync({
		rot13ServiceStatus: 400,
		message: "Unexpected status from ROT-13 service",
	});
});

Refactor the "simulates errors" test:

The assertFailureAsync() helper will need to support a rot13Client parameter.

Pass it through to transformAsync().

async function assertFailureAsync({
	rot13Client,
	port = 42,
	rot13ServiceStatus = VALID_ROT13_STATUS,
	rot13ServiceHeaders = VALID_ROT13_HEADERS,
	rot13ServiceBody = VALID_ROT13_BODY,
	message,
}) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			rot13Client,
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

TypeScript needs another type declaration:

async function assertFailureAsync({
	rot13Client,
	port = 42,
	rot13ServiceStatus = VALID_ROT13_STATUS,
	rot13ServiceHeaders = VALID_ROT13_HEADERS,
	rot13ServiceBody = VALID_ROT13_BODY,
	message,
}: {
	rot13Client?: Rot13Client,
	port?: number,
	rot13ServiceStatus?: number,
	rot13ServiceHeaders?: HttpHeaders,
	rot13ServiceBody?: string,
	message?: string,
}) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			rot13Client,
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

The test is ready to be refactored.

Extract the test-specific values into variables.

it("simulates errors", async () => {
	const rot13Client = Rot13Client.createNull([{ error: "my error" }]);

	const port = 9999;
	const rot13ServiceStatus = 500;
	const rot13ServiceHeaders = {};
	const rot13ServiceBody = "my error";
	const message = "Unexpected status from ROT-13 service";

	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			rot13Client,
			port: port,
		}),
		expectedError,
	);
});

Replace the newly-generic code.

No need to extract the code; just replace it with a call assertFailureAsync(). Don’t forget to await it.

it("simulates errors", async () => {
	const rot13Client = Rot13Client.createNull([{ error: "my error" }]);

	const port = 9999;
	const rot13ServiceStatus = 500;
	const rot13ServiceHeaders = {};
	const rot13ServiceBody = "my error";
	const message = "Unexpected status from ROT-13 service";

	await assertFailureAsync({
		port,
		rot13Client,
		rot13ServiceStatus,
		rot13ServiceHeaders,
		rot13ServiceBody,
		message,
	});
});

Inline and clean up.

it("simulates errors", async () => {
	const rot13Client = Rot13Client.createNull([{ error: "my error" }]);
	await assertFailureAsync({
		// port not relevant, and no longer needed
		rot13Client,
		rot13ServiceStatus: 500,
		rot13ServiceHeaders: {},
		rot13ServiceBody: "my error",
		message: "Unexpected status from ROT-13 service",
	});
});

Refactor the production code:

Prepare to extract the function by pulling out non-generic variables.

It’s already pretty generic. The only variable to pull out is the message.

async transformAsync(port, text, correlationId) {
	this._listener.emit({ port, text, correlationId });

	const response = await this._httpClient.requestAsync({
		host: HOST,
		port,
		method: "POST",
		path: "/rot13/transform",
		headers: {
			"content-type": "application/json",
			"x-correlation-id": correlationId,
		},
		body: JSON.stringify({ text }),
	});

	if (response.status !== 200) {
		const message = "Unexpected status from ROT-13 service";

		throw new Error(
			`${message}\n` +
			`Host: ${HOST}:${port}\n` +
			`Endpoint: ${TRANSFORM_ENDPOINT}\n` +
			`Status: ${response.status}\n` +
			`Headers: ${JSON.stringify(response.headers)}\n` +
			`Body: ${response.body}`
		);
	}

	const parsedBody = JSON.parse(response.body);
	return parsedBody.transformed;
}

You’re ready to extract out the helper function.

Call it throwError().

Inline and clean up.

export class Rot13Client {
	// ...

	async transformAsync(port, text, correlationId) {
		this._listener.emit({ port, text, correlationId });

		const response = await this._httpClient.requestAsync({
			host: HOST,
			port,
			method: "POST",
			path: "/rot13/transform",
			headers: {
				"content-type": "application/json",
				"x-correlation-id": correlationId,
			},
			body: JSON.stringify({ text }),
		});

		if (response.status !== 200) {
			throwError("Unexpected status from ROT-13 service", port, response);
		}

		const parsedBody = JSON.parse(response.body);
		return parsedBody.transformed;
	}

	// ...
}

function throwError(message, port, response) {
	throw new Error(
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		`Endpoint: ${TRANSFORM_ENDPOINT}\n` +
		`Status: ${response.status}\n` +
		`Headers: ${JSON.stringify(response.headers)}\n` +
		`Body: ${response.body}`,
	);
}

TypeScript needs type declarations in the extracted function:

function throwError(message: string, port: number, response: HttpClientResponse) {
	throw new Error(
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		`Endpoint: ${TRANSFORM_ENDPOINT}\n` +
		`Status: ${response.status}\n` +
		`Headers: ${JSON.stringify(response.headers)}\n` +
		`Body: ${response.body}`,
	);
}

Complete Solution

Test code:

describe.only("ROT-13 Service client", () => {
	// ...

	describe("failure paths", () => {

		it("fails gracefully when status code has unexpected value", async () => {
			await assertFailureAsync({
				rot13ServiceStatus: 400,
				message: "Unexpected status from ROT-13 service",
			});
		});

		// ...
	});


	describe("nulled instance", () => {
		// ...

		it("simulates errors", async () => {
			const rot13Client = Rot13Client.createNull([{ error: "my error" }]);
			await assertFailureAsync({
				rot13Client,
				rot13ServiceStatus: 500,
				rot13ServiceHeaders: {},
				rot13ServiceBody: "my error",
				message: "Unexpected status from ROT-13 service",
			});
		});

		// ...
	});

});
// ...

async function assertFailureAsync({
	rot13Client,
	port = 42,
	rot13ServiceStatus = VALID_ROT13_STATUS,
	rot13ServiceHeaders = VALID_ROT13_HEADERS,
	rot13ServiceBody = VALID_ROT13_BODY,
	message,
}) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			rot13Client,
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

Test code (TypeScript):

describe.only("ROT-13 Service client", () => {
	// ...

	describe("failure paths", () => {

		it("fails gracefully when status code has unexpected value", async () => {
			await assertFailureAsync({
				rot13ServiceStatus: 400,
				message: "Unexpected status from ROT-13 service",
			});
		});

		// ...
	});


	describe("nulled instance", () => {
		// ...

		it("simulates errors", async () => {
			const rot13Client = Rot13Client.createNull([{ error: "my error" }]);
			await assertFailureAsync({
				rot13Client,
				rot13ServiceStatus: 500,
				rot13ServiceHeaders: {},
				rot13ServiceBody: "my error",
				message: "Unexpected status from ROT-13 service",
			});
		});

		// ...
	});

});
// ...

async function assertFailureAsync({
	rot13Client,
	port = 42,
	rot13ServiceStatus = VALID_ROT13_STATUS,
	rot13ServiceHeaders = VALID_ROT13_HEADERS,
	rot13ServiceBody = VALID_ROT13_BODY,
	message,
}: {
	rot13Client?: Rot13Client,
	port?: number,
	rot13ServiceStatus?: number,
	rot13ServiceHeaders?: HttpHeaders,
	rot13ServiceBody?: string,
	message?: string,
}) {
	const expectedError =
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		"Endpoint: /rot13/transform\n" +
		`Status: ${rot13ServiceStatus}\n` +
		`Headers: ${JSON.stringify(rot13ServiceHeaders)}\n` +
		`Body: ${rot13ServiceBody}`;

	await assert.throwsAsync(
		() => transformAsync({
			rot13Client,
			port,
			rot13ServiceStatus,
			rot13ServiceHeaders,
			rot13ServiceBody,
		}),
		expectedError,
	);
}

Production code (JavaScript):

export class Rot13Client {
	// ...

	async transformAsync(port, text, correlationId) {
		this._listener.emit({ port, text, correlationId });

		const response = await this._httpClient.requestAsync({
			host: HOST,
			port,
			method: "POST",
			path: "/rot13/transform",
			headers: {
				"content-type": "application/json",
				"x-correlation-id": correlationId,
			},
			body: JSON.stringify({ text }),
		});

		if (response.status !== 200) {
			throwError("Unexpected status from ROT-13 service", port, response);
		}

		const parsedBody = JSON.parse(response.body);
		return parsedBody.transformed;
	}

	//...
}

function throwError(message, port, response) {
	throw new Error(
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		`Endpoint: ${TRANSFORM_ENDPOINT}\n` +
		`Status: ${response.status}\n` +
		`Headers: ${JSON.stringify(response.headers)}\n` +
		`Body: ${response.body}`,
	);
}

Production code (TypeScript):

export class Rot13Client {
	// ...

	async transformAsync(
		port: number,
		text: string,
		correlationId: string,
	): Promise<string> {
		this._listener.emit({ port, text, correlationId });

		const response = await this._httpClient.requestAsync({
			host: HOST,
			port,
			method: "POST",
			path: "/rot13/transform",
			headers: {
				"content-type": "application/json",
				"x-correlation-id": correlationId,
			},
			body: JSON.stringify({ text }),
		});

		if (response.status !== 200) {
			throwError("Unexpected status from ROT-13 service", port, response);
		}

		const parsedBody = JSON.parse(response.body);
		return parsedBody.transformed;
	}

	//...
}

function throwError(message: string, port: number, response: HttpClientResponse) {
	throw new Error(
		`${message}\n` +
		`Host: ${HOST}:${port}\n` +
		`Endpoint: ${TRANSFORM_ENDPOINT}\n` +
		`Status: ${response.status}\n` +
		`Headers: ${JSON.stringify(response.headers)}\n` +
		`Body: ${response.body}`,
	);
}

Next challenge

Return to module overview