Skip to content

status codes refactor + src files export modified#1

Open
muhayli wants to merge 3 commits intoMarkonis:mainfrom
muhayli:main
Open

status codes refactor + src files export modified#1
muhayli wants to merge 3 commits intoMarkonis:mainfrom
muhayli:main

Conversation

@muhayli
Copy link

@muhayli muhayli commented Jan 27, 2023

src files export modified + status codes refactor + updated the readMe.md

export const HTTP_VERSION_NOT_SUPPORTED: HttpStatus = { code: 505, phrase: "HTTP Version Not Supported" };
export const INSUFFICIENT_STORAGE: HttpStatus = { code: 507, phrase: "Insufficient Storage" };
export const NETWORK_AUTHENTICATION_REQUIRED: HttpStatus = { code: 511, phrase: "Network Authentication Required" };
export abstract class HttpStatusCode {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, what's the benefit of having a class vs a simple module?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class format could be better for encapsulation and organization.

import { deepStrictEqual } from "assert";
import { listener } from "../src/listener";
import { INTERNAL_SERVER_ERROR, OK } from "../src/status";
import { HttpStatusCode } from "../src/status";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the purpose of having the module is just to have a namespace for status codes, you can always import like this, without having to add a custom class.

import * as HttpStatusCode from "../src/status"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely correct, and the only reasons I designed it this way were for potential future improvements/updates and to lessen the number of imports when using more than one status code in a single file, and because the class structure might allow for greater encapsulation and organization.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate your effort here, and I understand your point about reducing the number of potential imports / exports in a file. Can this be solved in the same way that you cleaned up the exports in the index.ts?

Something like this maybe?

export * as HttpStatusCode from "./status";

I'm just cautious about using classes as they come with some more baggage, that in this particular case we can probably get away without.

What do you think?

Copy link
Author

@muhayli muhayli Feb 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you're right, with this type of export/import
export * as HttpStatusCode from "./status";
The class doesn't seem to be necessary. since reducing import/export was my primary motivation for taking the class, and this appears to have fixed it.

Copy link
Owner

@Markonis Markonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work cleaning up the imports an exports 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants