10 naming #25

IK541 · 2024-09-20T08:34:38Z

Changed notes directory to note directory as all the other resource directories (event, search, task, user) use singular
Modified controller methods names so that these follow the same convention across different resources (only event, task and note have methods with similar functionality, search and users were left untouched as these have completly different functionality, therefore there was no need for same naming conventions and their current naming conventions seemed reasonable). The new naming convention is:

html request method	endpoint name	controller method name	service method name
`GET`	`<resource>`	`getUser<Resource>s`	`fetchUser`
`GET`	`<resource>/<id>`	`get<Resource>ById`	`fetch`
`POST`	`<resource>`	`add<Resource>`	`add`
`PATCH`	`<resource>/<id>`	`edit<Resource>`	`edit`
`DELETE`	`<resource>/<id>`	`delete<Resource>`	`delete`

Modified task endpoints and one of note endpoints to adhere to the naming convention
Modified service methods names to adhere to the naming conventions - mainly deleting resource name from method name as resource names are already included in service class names
added typing to notes

smazik02

Looks good overall, I'd suggest one naming convention change though

smazik02 · 2024-09-20T10:02:09Z

src/resource/event/event.service.ts

@@ -42,7 +42,7 @@ export class EventService {
        return event;
    }

-    async fetchUserEvents(userId: number): Promise<ReturnEventDTO[]> {
+    async fetchUser(userId: number): Promise<ReturnEventDTO[]> {


Wrong naming convention in my opinion, service method returns Events, not a User, I think something like "fetchByUser" would be more descriptive

smazik02 · 2024-09-20T10:02:26Z

src/resource/note/note.service.ts

@@ -12,22 +12,22 @@ export class NoteService {
        private noteRepository: Repository<NoteEntity>,
    ) {}

-    async fetchUserNotes(user: UserEntity) {
+    async fetchUser(user: UserEntity): Promise<NoteDTO[]> {


smazik02 · 2024-09-20T10:05:24Z

src/resource/task/task.service.ts

        return await this.taskRepository.findOne({
            where: { id, user },
        });
    }

-    async getAllUserTasks(user: UserEntity): Promise<ReturnTaskDTO[]> {
+    async fetchUser(user: UserEntity): Promise<ReturnTaskDTO[]> {


IK541 · 2024-09-20T11:37:51Z

The requested change (renaming fetchUser to fetchByUser) has been implemented. The updated naming table looks like this:

html request method	endpoint name	controller method name	service method name
`GET`	`<resource>`	`getUser<Resource>s`	`fetchByUser`
`GET`	`<resource>/<id>`	`get<Resource>ById`	`fetch`
`POST`	`<resource>`	`add<Resource>`	`add`
`PATCH`	`<resource>/<id>`	`edit<Resource>`	`edit`
`DELETE`	`<resource>/<id>`	`delete<Resource>`	`delete`

smazik02

Looks good to me

Aeri4a

Small changes, overall good job

Aeri4a · 2024-09-20T19:14:15Z

src/resource/event/event.controller.ts

@@ -54,7 +54,7 @@ export class EventController {
    async getUserEvents(
        @GetUser('id') userId: number,
    ): Promise<ReturnEventDTO[]> {
-        return await this.eventService.fetchUserEvents(userId);
+        return await this.eventService.fetchByUser(userId);


And that one is clear, cool

Aeri4a · 2024-09-20T19:15:37Z

src/resource/event/event.controller.ts

@@ -74,7 +74,7 @@ export class EventController {
    @Get(':id')
    @GetEventByIdApi()
    async getEventById(@Param('id') eventId: number): Promise<ReturnEventDTO> {
-        return await this.eventService.fetchById(eventId);
+        return await this.eventService.fetch(eventId);


fetchById because we don't know if we fetch all or not.
You could use [serviceName].fetchAll or fetchOne and albo fetchById

Aeri4a · 2024-09-20T19:16:53Z

src/resource/event/event.controller.ts

    }

    @HttpCode(HttpStatus.NO_CONTENT)
    @Delete('exceptions/:id')
    @DeleteExceptionApi()
    async deleteException(@Param('id') exceptionId: number): Promise<void> {
-        await this.eventService.removeExceptionById(exceptionId);
+        await this.eventService.deleteException(exceptionId);


deleteException in this case is clear, because we know that we want to delete one so it is obvious that we need for example id

Aeri4a · 2024-09-20T19:17:50Z

src/resource/note/note.controller.ts

+    EditNoteApi as EditNoteApi,
+    GetUserNotesApi as GetUserNotesApi,


Wait, why there is 'as'?

Aeri4a · 2024-09-20T19:20:12Z

src/resource/note/note.controller.ts

+    @GetUserNotesApi()
+    getUserNotes(@GetUser() user: UserEntity): Promise<NoteDTO[]> {
+        return this.notesService.fetchByUser(user);


I forgot to mention that if TypeScript infer type properly if you hover your mouse on function then you don't need to declare type explicitly

Aeri4a · 2024-09-20T19:21:27Z

src/resource/task/task.controller.ts

+    GetUserTasksApi as GetUserTasksApi,
+    GetTaskByIdApi as GetTaskbyIdApi,


Again unnecessary 'as'

IK541 · 2024-09-20T20:18:14Z

Both changes have been implemented: fetch changed to fetchById and as removed
The updated naming table looks like this:

html request method	endpoint name	controller method name	service method name
`GET`	`<resource>`	`getUser<Resource>s`	`fetchByUser`
`GET`	`<resource>/<id>`	`get<Resource>ById`	`fetchById`
`POST`	`<resource>`	`add<Resource>`	`add`
`PATCH`	`<resource>/<id>`	`edit<Resource>`	`edit`
`DELETE`	`<resource>/<id>`	`delete<Resource>`	`delete`

IK541 added 6 commits September 19, 2024 13:33

changed notes to note

3d872c5

unified controller naming

a9d954f

changed task endpoint naming

ce9cc38

added types to notes

481c5da

changed service methods names

b0de70a

fixed note post endpoint

c4176ce

smazik02 requested changes Sep 20, 2024

View reviewed changes

changed fetchUser in services to fetchByUser

5c90e51

smazik02 reviewed Sep 20, 2024

View reviewed changes

Aeri4a requested changes Sep 20, 2024

View reviewed changes

IK541 added 2 commits September 20, 2024 21:38

removed unnecessary as

a2d989b

added ById to single fetches

edbd119

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

10 naming #25

10 naming #25

IK541 commented Sep 20, 2024

smazik02 left a comment

smazik02 Sep 20, 2024

Aeri4a Sep 20, 2024

smazik02 Sep 20, 2024

smazik02 Sep 20, 2024

IK541 commented Sep 20, 2024

smazik02 left a comment

Aeri4a left a comment

Aeri4a Sep 20, 2024

Aeri4a Sep 20, 2024

Aeri4a Sep 20, 2024

Aeri4a Sep 20, 2024

Aeri4a Sep 20, 2024

Aeri4a Sep 20, 2024

IK541 commented Sep 20, 2024

		EditNoteApi as EditNoteApi,
		GetUserNotesApi as GetUserNotesApi,

		GetUserTasksApi as GetUserTasksApi,
		GetTaskByIdApi as GetTaskbyIdApi,

10 naming #25

Are you sure you want to change the base?

10 naming #25

Conversation

IK541 commented Sep 20, 2024

smazik02 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IK541 commented Sep 20, 2024

smazik02 left a comment

Choose a reason for hiding this comment

Aeri4a left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IK541 commented Sep 20, 2024