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.

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

✨Refactor BaseEntity to Support Multiple Id Types #490

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

neozhu
Copy link
Owner

@neozhu neozhu commented Aug 25, 2023

Description:

Following suggestions from several developers regarding the need to support common Entity Id types in EntityFramework, such as Guid, String, and Numeric, I've made an attempt to enhance the flexibility while ensuring simplicity.

Traditionally, I've always relied on int auto-increment types as primary keys. This is primarily due to their straightforward nature and ease of sorting. However, to cater to broader needs, this PR introduces a way to seamlessly support other types without a significant overhaul of the existing project.

Changes Made:

  1. Introduced a new interface IEntity<T> inheriting from IEntity:
public interface IEntity<T> : IEntity
{
    T Id { get; set; }
}
  1. Refactored BaseEntity to inherit from IEntity<T>, specifying the desired primary key type:
public abstract class BaseEntity: IEntity<int>
{
    public virtual int Id { get; set; }
    //... other properties and methods as before
}

To use a different type like string for the Id, simply modify the BaseEntity class definition to IEntity<string>. Remember to run the add-migration command afterward.

Note: It's advisable not to change the primary key type mid-project development.


This description offers a clear and concise explanation of the changes you've made. Remember to adjust as needed, depending on your project's PR review process and conventions.

@neozhu neozhu added Enhancement New feature or request Refactoring Improvement to the existing code base labels Aug 25, 2023
@neozhu neozhu requested a review from Bram1903 August 25, 2023 23:38
@neozhu neozhu linked an issue Aug 25, 2023 that may be closed by this pull request
@Bram1903 Bram1903 marked this pull request as ready for review August 25, 2023 23:45
Copy link
Collaborator

@Bram1903 Bram1903 left a comment

Choose a reason for hiding this comment

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

Looks good! Personally I don't really feel the need for supporting GUID's, since the performance hit and actual benefits are in our case probably unnecessary, but it's good to at least have support for it!

Btw @neozhu, next time you use ChatGPT make sure to remove the part where it tells you to make changes to fit your specific use case 😉

Nothing wrong with generating your PR btw, since it's a lot easier to read than normally! No offence meant 😂

@Bram1903 Bram1903 merged commit f0ac6fc into main Aug 25, 2023
2 checks passed
@neozhu
Copy link
Owner Author

neozhu commented Aug 26, 2023

It should be evening/early morning for you now?

@Bram1903
Copy link
Collaborator

It should be evening/early morning for you now?

At the time it was 02:00 after midnight 😂. Weekend has begun for me, and I was playing some games with friends, and just when I went to bed I saw your PR, so I thought let's review it right away 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Refactoring Improvement to the existing code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

identity id as Guid instead of string
2 participants