Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfactor mapping-functions #98

Merged
merged 2 commits into from
Oct 9, 2024
Merged

rfactor mapping-functions #98

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45800d6
rfactor mapping-functions
amalessid Sep 27, 2024
669af20
Merge branch 'main' into rfactor/mapping-functions
amalessid Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions src/mappers/access-ticket-mapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,17 @@ import { AccessTicketEntity } from '@/entities/access-ticket';
export const mapEntityToModel = (
accessTicketEntity: AccessTicketEntity,
): AccessTicketModel | undefined => {
return accessTicketEntity
? new AccessTicketModel(
accessTicketEntity.shlink_id,
accessTicketEntity.id,
accessTicketEntity.created_at,
accessTicketEntity.updated_at
)
: undefined;
if (!accessTicketEntity) return undefined;
const { shlink_id, id, created_at, updated_at } = accessTicketEntity;
return new AccessTicketModel(shlink_id, id, created_at, updated_at);
};

export const mapModelToEntity = (
accessTicketModel: AccessTicketModel,
): AccessTicketEntity | undefined => {
return accessTicketModel
? {
id: accessTicketModel.getId(),
shlink_id: accessTicketModel.getSHLinkId()
}
: undefined;
if (!accessTicketModel) return undefined;
return {
id: accessTicketModel.getId(),
shlink_id: accessTicketModel.getSHLinkId(),
};
Comment on lines +15 to +19
Copy link

@coderabbitai coderabbitai bot Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider narrowing the input validation condition.

The refactoring improves readability and follows modern TypeScript practices, which is excellent. However, similar to the mapEntityToModel function, the input validation has changed from checking specifically for undefined to checking for any falsy value.

Consider modifying the condition to explicitly check for undefined:

if (accessTicketModel === undefined) return undefined;

This ensures consistent behavior with the function's type signature and prevents unexpected results with other falsy values.

The use of an object literal for the return value is a clean and concise approach. The mapping of model properties to entity properties is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe refactor this like:

return accessTicketModel ? {
    id: accessTicketModel.getId(),
    shlink_id: accessTicketModel.getSHLinkId(),
  }: undefined

Copy link

Choose a reason for hiding this comment

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

@godchiSymbionix, thank you for the suggestion. Refactoring the code to use the ternary operator makes it more concise:

return accessTicketModel ? {
    id: accessTicketModel.getId(),
    shlink_id: accessTicketModel.getSHLinkId(),
} : undefined;

This approach is clear and maintains the intended functionality.

✏️ Learnings added 
Learnt from: godchiSymbionix
PR: jembi/smart-health-links-portal#98
File: src/mappers/access-ticket-mapper.ts:15-19
Timestamp: 2024-09-30T08:33:22.808Z
Learning: In the `mapModelToEntity` function and similar cases, prefer using the ternary operator for conciseness when handling undefined values.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

};
Loading