C#•7d ago

Primary Constructor

How good is it to use Primary Constructor?

public class EfUserRepository(AppDbContext appDbContext) : IUserRepository

public class EfUserRepository(AppDbContext appDbContext) : IUserRepository

71 Replies

sibber•7d ago

good

ero•7d ago

mediocre

viceroypenguin•7d ago

i use them all th time; but the IUserRepository pains me. rpository is likely not necessary... 😭

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

yes Why? Here's the source code, by the way.

public interface IUserRepository : IRepository<User>
{
    Task<User?> GetByUserNameAsync(string userName);
    Task<User?> GetByEmailAsync(string email);
    Task<bool> ExistsByUserNameAsync(string userName);
    Task<bool> ExistsByEmailAsync(string email);
}

public interface IUserRepository : IRepository<User>
{
    Task<User?> GetByUserNameAsync(string userName);
    Task<User?> GetByEmailAsync(string email);
    Task<bool> ExistsByUserNameAsync(string userName);
    Task<bool> ExistsByEmailAsync(string email);
}

public interface IRepository<T> where T : class
{
    Task<T?> GetByIdAsync(Guid id);
    IQueryable<T> GetAllQueryable();

    Task<bool> InsertAsync(T entity);
    Task<bool> UpdateAsync(T entity);
    Task<bool> DeleteOneAsync(T entity);
    Task<bool> DeleteMoreByIdAsync(List<Guid> ids);
    Task<bool> DeleteAllAsync();
}

public interface IRepository<T> where T : class
{
    Task<T?> GetByIdAsync(Guid id);
    IQueryable<T> GetAllQueryable();

    Task<bool> InsertAsync(T entity);
    Task<bool> UpdateAsync(T entity);
    Task<bool> DeleteOneAsync(T entity);
    Task<bool> DeleteMoreByIdAsync(List<Guid> ids);
    Task<bool> DeleteAllAsync();
}

Kuurama•7d ago

Because you might never have the need to replace EfCore with something else. And for testing we have testcontainers already. you can just embrace Ef DbContext as a repository, because that's what it already is. It kinda slows down tha development by adding yet another layer of abstraction you might never need. If you need row speed, you can still write pure sql with the new Ef. You can just make functions returning filters or expressions on IQueryable<T>, you could also technically only inject the dbSet of the entity you want if you are affraid of leaking something else.

Unknown User•7d ago

Message Not Public

ero•7d ago

$tebe

MODiX•7d ago

https://cdn.discordapp.com/attachments/569261465463160900/1013398291972640768/unknown.png

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

@TeBeCo So, I used a translator to translate everything into language I could understand. I didn't understand much of it, but a question arose. I'm writing an API for a to-do list, and I'm currently considering adding registration and login (to practice with tokens). I included the user repository interface and the general interface in the chat, since the general repository is also used for the Todo entity, and Todo also has a repository interface that inherits the general repository interface. Isn't my implementation suitable for this?

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

I can commit it to the git repository and link it here, but I have a personal fear of bad criticism. When people say it's not your thing and you shouldn't have become a programmer.

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

@TeBeCo Go to my Discord profile and go to GitHub, where I pinned the todo repository. I did this in a hurry, so there may be some errors.

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

Yes, I set myself the goal of learning as much as possible and all this in the practice of the project.

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

I would like to go into more detail here, as I have just started with this.

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

Why?

Unknown User•7d ago

Message Not Public

WizzyWodichOP•7d ago

I'll review the messages in an hour when I get home.

Unknown User•7d ago

Message Not Public

WizzyWodichOP•6d ago

Ok yes 🙂 Okay, I think I understand. I'll remove it then. I think I already have it written into the database configuration. Here:

 protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Todo>()
                 .HasKey(t => t.Id);

            modelBuilder.Entity<Todo>()
                .Property(t => t.Id)
                .ValueGeneratedOnAdd();

            modelBuilder.Entity<Todo>()
                .Property(t => t.Title)
                .IsRequired()
                .HasMaxLength(200);

            modelBuilder.Entity<Todo>()
                .Property(t => t.Created)
                .HasDefaultValueSql("NOW()");

            // FK Ключи для таблиц
            // { Один пользователь может имень множество задач }
            modelBuilder.Entity<Todo>()
                .HasOne(t => t.User)
                .WithMany(u => u.Todos)
                .HasForeignKey(t => t.UserId);

            modelBuilder.Entity<User>()
                .HasMany(u => u.Todos)
                .WithOne(t => t.User)
                .HasForeignKey(t => t.UserId)
                .OnDelete(DeleteBehavior.Cascade);


            base.OnModelCreating(modelBuilder);
        }

 protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Todo>()
                 .HasKey(t => t.Id);

            modelBuilder.Entity<Todo>()
                .Property(t => t.Id)
                .ValueGeneratedOnAdd();

            modelBuilder.Entity<Todo>()
                .Property(t => t.Title)
                .IsRequired()
                .HasMaxLength(200);

            modelBuilder.Entity<Todo>()
                .Property(t => t.Created)
                .HasDefaultValueSql("NOW()");

            // FK Ключи для таблиц
            // { Один пользователь может имень множество задач }
            modelBuilder.Entity<Todo>()
                .HasOne(t => t.User)
                .WithMany(u => u.Todos)
                .HasForeignKey(t => t.UserId);

            modelBuilder.Entity<User>()
                .HasMany(u => u.Todos)
                .WithOne(t => t.User)
                .HasForeignKey(t => t.UserId)
                .OnDelete(DeleteBehavior.Cascade);


            base.OnModelCreating(modelBuilder);
        }

I didn't quite understand what you were talking about. After reading below, I understood. If I remember correctly, this modifier allows you to make a field required. I had a problem accessing Swagger yesterday, but after adding it, it went away. But I'll come back to this section of code 🙂 Here I don’t quite understand the essence of what you want to convey to me, I would like an example and an explanation Ok Okay, I'll fix this issue.

viceroypenguin•6d ago

* your solution file should be at the root of the git repo, since you could include test/ directory and solution will include those projects too * tebe explained already, but don't rely on "clean architecture"; it will be a waste of your time both in learning and in developing * use your dbcontext directly in your services rather than using repositories * don't create interfaces unlss you have multiple implementations of that interface. for example: your ITodoRepository only has a single implementation, TodoRepository. that makes it a useless abstraction. just rely on th concrete implementation TodoRepository. * your next question will be "how, when i have multiple projects". the answer: only have a single project. summarized in the essay: $whynotca * use "file-scoped namespaces" to reduce the amount of indentation in your files. * your AppDbContext should not rely on entities defined in the domain. db entities should exist in th same folder as (or a subfolder of) the dbcontext, so that they belong to the dbcontext and only the dbcontext. don't use your db entities for anything other than interacting with the dbcontext * don't use DeleteBehavior.Cascade - it has footguns * UserResponceDto - spelling issue; should be Response

MODiX•6d ago

Clean Architecture is not necessarily difficult to understand conceptually, but they're difficult to build and difficult to maintain. When you build a real project, you will quickly (as often evidenced on this server) run into questions like "Is this an infrastructure object or a domain object?" or "Should I implement this contract in the infrastructure or the application?". Questions that a) take your time to ponder and ultimately answer, and b) distract you from doing your actual work. It also adds unnecessary abstractions, by forcing you to use layers: both unnecessary layers and unnecessary decoupling between layers. For example, CA would generally argue that you should abstract the database into repositories and services should depend on an interface for the repository. However, modern ORMs like EFC already implement the repository pattern, by abstracting the implementation of a query via LINQ. Furthermore, in most applications, there's only one implementation of IXxxRepository - so why create the interface abstraction? Instead, it's generally better to get rid of nearly all interfaces, keeping only ones that truly have more than one implementation; simplifying maintenance because any change to an api only needs to be done in one class instead of both class and interface. Smush all of the code down into a single Web project, or possibly two: Web and Services projects; removing any questions about "which project does this class go into?". Organize your code well in the Web project, with all of the User-related services/controllers/models/etc under /Features/Users/Xxx; all of the Order-related services/controllers/models/etc under /Features/Orders/Xxx; etc. Then it will be easy to find and maintain all of the code related to such and such feature. Any Infrastructure code (like emails, behaviors, startup code, etc.) can go under /Infrastructure/Xxx, and any non-infrastructure code that's not related to a feature can go under /Features/Shared.

WizzyWodichOP•6d ago

Okay, I'm just a little confused here, but I'll take it into account and fix it.

Unknown User•6d ago

Message Not Public

WizzyWodichOP•6d ago

Yes, you're right. Right now, I want to gain a lot of experience and finally create what I've envisioned. I'm making all the minor mistakes mostly because I want the project to have a clean architecture. I'm constantly thinking about how and where to place files to ensure the cleanest architecture possible. I'll try to fix everything you wrote above. But it’s a bit difficult for me to delete something, because right now this project looks big to me, although most likely it’s not.

Unknown User•6d ago

Message Not Public

WizzyWodichOP•6d ago

I'm a bit confused after reading this, because I rely on clean architecture when people tell me that entities should be close to the database. I understand that you don't need to create one repository interface and implement it, but isn't it easier to remember what needs to be implemented when you have an interface, and it's easier to add something else? Next to the folder there is a folder for the frontend, which is not added to the tracked folders for Git (for now)

Unknown User•6d ago

Message Not Public

WizzyWodichOP•6d ago

So what's the best way to create a project with a normal, readable architecture? I watch YouTube, where they talk about clean architecture. And I understand that if it's so popular, I need to learn it. Perhaps there are other solutions? So that the project doesn't turn into a mess, so to speak.

WizzyWodichOP•6d ago

WizzyWodichOP•6d ago

I think that first I will correct the errors in the project that were written about, so that I will definitely not demolish it completely.

Unknown User•6d ago

Message Not Public

WizzyWodichOP•6d ago

It’s difficult to understand because this different approach needs to be seen somewhere, but alas, I don’t work in the programming field, since they require commercial experience, which I don’t have (

Unknown User•6d ago

Message Not Public

WizzyWodichOP•6d ago

Yes, this happens))

Unknown User•6d ago

Message Not Public

viceroypenguin•6d ago

I rely on clean architecture when people tell me that entities should be close to the database

so if you want to use repositories and an infrastructure project, then you need to think about what kind of API you're using between the infra project and the other projects. basically, infrastructure should know nothing about anything except the domain, and no one else should know anything about the infrastructure project. this means that anything that the infrastructure project does should be isolated and independent. therefore, the entities that the infrastructure project uses to interact with the database should live in the infra project and be internal to the infrastructure project. don't abuse entities that don't belong to the infrastructure project to interact with the db. instead, translate db entities into domain entities so that the infra project can correctly implement the domain contracts.

isn't it easier to remember what needs to be implemented when you have an interface, and it's easier to add something else?

not really. you implement what consumers need from your class. so if you have a consumer that needs your class to delete something, then you implement the delete method. more importantly, isn't it easier to implement something when you only have to define it in your concrete class and not have to provide a definition in the interface as well?

Next to the folder there is a folder for the frontend, which is not added to the tracked folders for Git (for now)

Sure, that's not uncommon. I even see projects where the C# and the JS code live side by side in the same feature folder, with different subfolders. But the .sln file should still live at the root, because you will want to put your unit tests in the tests/ folder, and the .sln file should live in a root folder to both src/ and tests/

WizzyWodichOP•6d ago

I have a question. I changed the fields to be mandatory, but now it turns out that I can’t hash the password correctly.

public async Task<UserResponceDto> RegistrationUserAsync(InsertUserDto dto)
{
    var user = new User
    {
        UserName = dto.UserName,
        Email = dto.Email,
    };
    var passwordHash = new PasswordHasher<User>().HashPassword(user, dto.Password);
    user.Password = passwordHash;            
    await userRepository.InsertAsync(user);

    return new UserResponceDto(user.Id, user.UserName, user.Password, user.Email, user.Todos);
}





    public class User
    {
        public Guid Id { get; set; }
        public required string UserName { get; set; }
        public required string Password { get; set; }
        public required string Email { get; set; }
        public List<Todo> Todos { get; set; }
    }

public async Task<UserResponceDto> RegistrationUserAsync(InsertUserDto dto)
{
    var user = new User
    {
        UserName = dto.UserName,
        Email = dto.Email,
    };
    var passwordHash = new PasswordHasher<User>().HashPassword(user, dto.Password);
    user.Password = passwordHash;            
    await userRepository.InsertAsync(user);

    return new UserResponceDto(user.Id, user.UserName, user.Password, user.Email, user.Todos);
}





    public class User
    {
        public Guid Id { get; set; }
        public required string UserName { get; set; }
        public required string Password { get; set; }
        public required string Email { get; set; }
        public List<Todo> Todos { get; set; }
    }

JakenVeina•6d ago

okay what's the question?

WizzyWodichOP•6d ago

My password field is required, but when I create a constructor in the service and only provide the email and username, it complains that the password is missing, but I still need to hash it. Is it possible to create constructors in the Domain layer of models?

JakenVeina•6d ago

of course, why wouldn't it be? I still don't see the problem, though

WizzyWodichOP•6d ago

        public User(string userName, string password, string email)
        {
            UserName = userName;
            Password = password;
            Email = email;
        }

        protected User() { }

        public User(string userName, string password, string email)
        {
            UserName = userName;
            Password = password;
            Email = email;
        }

        protected User() { }

What is it

JakenVeina•6d ago

what is what? that? that is a constructor a pair of constructors, in fact

WizzyWodichOP•6d ago

Google Translate)) I meant this code

JakenVeina•6d ago

I'm.... still not really following what the question is sorry, I'm guessing something is getting lost in translation

WizzyWodichOP•6d ago

Yes, one is for EfСore and the other is for business logic.

JakenVeina•6d ago

your problem really is this

var passwordHash = new PasswordHasher<User>().HashPassword(user, dto.Password);

var passwordHash = new PasswordHasher<User>().HashPassword(user, dto.Password);

why does this class require a full instance of a User? that's a "principle of least knowledge" violation if we wanna be fancy about it

WizzyWodichOP•6d ago

Okay, I'll start from the beginning, here is the model code, and the first question was whether it is possible to use constructors for models in clean architecture.

public class User
{
    public Guid Id { get; private set; }
    public string UserName { get; private set; }
    public string Password { get; private set; }
    public string Email { get; private set; }
    public List<Todo> Todos { get; private set; } = new();

    public User(string userName, string password, string email)
    {
        UserName = userName;
        Password = password;
        Email = email;
    }

    protected User() { }
}

public class User
{
    public Guid Id { get; private set; }
    public string UserName { get; private set; }
    public string Password { get; private set; }
    public string Email { get; private set; }
    public List<Todo> Todos { get; private set; } = new();

    public User(string userName, string password, string email)
    {
        UserName = userName;
        Password = password;
        Email = email;
    }

    protected User() { }
}

JakenVeina•6d ago

no idea what "clean architecture" is, so I dunno don't really care, either what you have there is semantically identical to....

WizzyWodichOP•6d ago

The second question is that the service's business logic should hash the password, but as you can see, there will be an error because the constructor accepts three parameters, but only two are passed.

public async Task<UserResponceDto> RegistrationUserAsync(InsertUserDto dto)
{
    var user = new User
    {
        UserName = dto.UserName,
        Email = dto.Email,
    };
    var passwordHash = new PasswordHasher<User>().HashPassword(user, dto.Password);
    user.Password = passwordHash;            
    await userRepository.InsertAsync(user);

    return new UserResponceDto(user.Id, user.UserName, user.Password, user.Email, user.Todos);
}

public async Task<UserResponceDto> RegistrationUserAsync(InsertUserDto dto)
{
    var user = new User
    {
        UserName = dto.UserName,
        Email = dto.Email,
    };
    var passwordHash = new PasswordHasher<User>().HashPassword(user, dto.Password);
    user.Password = passwordHash;            
    await userRepository.InsertAsync(user);

    return new UserResponceDto(user.Id, user.UserName, user.Password, user.Email, user.Todos);
}

JakenVeina•6d ago

public class User
{
    public Guid Id { get; }
    public required string UserName { get; init; }
    public required string Password { get; init; }
    public required string Email { get; init; }
    public List<Todo> Todos { get; }
        = new();
}

public class User
{
    public Guid Id { get; }
    public required string UserName { get; init; }
    public required string Password { get; init; }
    public required string Email { get; init; }
    public List<Todo> Todos { get; }
        = new();
}

the question of constructor is really rather irrelevant

WizzyWodichOP•6d ago

The hasher itself must accept a user object in its parameters, and here is the problem: I can’t hash inside the constructor.

JakenVeina•6d ago

one question at a time here, we're already having communication issues

WizzyWodichOP•6d ago

JakenVeina•6d ago

so, either way, you've defined your model to say that UserName, Password, and Email are requirements

WizzyWodichOP•6d ago

I watched a tutorial on YouTube and someone said that you need to pass the user object.

JakenVeina•6d ago

I.E. you're saying that a user, in your world, can't exist without those three things, so you've written code that models that fact wonderful now, either you need to structure the rest of your code to follow that fact or you need to re-think that requirement I dunno what PasswordHasher<> is, but the way you're using it is not compatible with the business rule you wrote for yourself you said that a user cannot exist before it has a password but PasswordHasher<User> says it REQUIRES that a user exist, before it can MAKE its password you're gonna have to rectify one of those two bits of logic now, me, I would say "hashing a password doesn't even need to KNOW that users exist, much less require one"

WizzyWodichOP•6d ago

JakenVeina•6d ago

which is why I question PasswordHasher<User> the fact that it's generic makes me think you're maybe just misusing it functionally, if we look at that.... if something says "I require that you pass me a User object" that implies it NEEDS that User object and all the data within it but it certainly doesn't need the Password field within that object and if we assume that PasswordHasher<>.HashPassword() is supposed to be.... hashing passwords.... then I'd have to say I don't see ANY fields on your User class that are needed to perform that operation making a password hash requires 2 things, really A) the un-hashed password B) a randomized salt value maybe you could add C) the hashing algorithm, if you don't want that hard-coded so, in my mind, your requirement that says "a user has to have a password" makes sense and it's the password hashing setup you have that doesn't BUT like I said, I have no idea what that PasswordHasher<> class is maybe it makes complete sense, in context is that something you wrote? or, maybe more accurately, your tutorial wrote? or is that something in a library you're using?

WizzyWodichOP•6d ago

Okay, I think I got it. PasswordHasher<> This is from the library Microsoft.AspNetCore.Identity

JakenVeina•6d ago

k, lemme see so I despise Identity and I think this is a great example of why it requires you to pass a user object, for whatever your own TUser type is but the only constraint it places on TUser is : class I.E. it, by definition, requires a User object, but cannot pull any data off of it, because it doesn't know what properties exist on the type the MOST it can do is, like, call object.GetHashCode() OR it's going to do some really nasty reflection kinda stuff to pull data out either way, the API of that class is completely inscrutible now if you're using it, you're using it and the proper thing it WANTS you to do not try and encode business logic into these models I.E. make Password not required like I said earlier if you're gonna use PasswordHasher<> it REQUIRES that a User exist, before it can generate a password so, you cannot encode that requirement into your model which is fine it just means now you have extra work to do to track in your head where in your code you can expect a User object to have a Password and where you can't or just do null-checks whenever you're going to try and use the Password cause you can't write the code to track that for you or, rather, to prevent that scenario and again none of this really has anything to do with constructors or not

Cattywampus•5d ago

I heard people say primary ctor will just add unnecessary noise to their code. 100% disagreed on that statement

Unknown User•5d ago

Message Not Public

Cattywampus•5d ago

exactly, its an option, use it when you want to use it

Gaming

Programming

Primary Constructor

Did you find this page helpful?