Skip to main content
Bounty Awarded with 50 reputation awarded by Just code
added 1 character in body
Source Link
Heslacher
  • 51k
  • 5
  • 83
  • 177

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more spcificspecific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post more code but its design doesn't look right.

It's difficult to come up with an example. The only thinkthing I would change about your code in the first place would be to use the repository instead of the invalid unit-of-work. So instead:

public class CountriesController : BaseController
{
  private IUnitOfWork unitOfWork;

  public CountriesController(iMSTUnitOfWork _unitOfWork)
  {
      unitOfWork = _unitOfWork;
  }
}

you'll have:

public class CountriesController : BaseController
{
    private readonly IRepository _repository;

    public CountriesController(IRepository repository)
    {
        _repository = repository;
    }
}

It might be a good idea to ask another question about your unit-of-work and repository that you use here.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more spcific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post more code but its design doesn't look right.

It's difficult to come up with an example. The only think I would change about your code in the first place would be to use the repository instead of the invalid unit-of-work. So instead:

public class CountriesController : BaseController
{
  private IUnitOfWork unitOfWork;

  public CountriesController(iMSTUnitOfWork _unitOfWork)
  {
      unitOfWork = _unitOfWork;
  }
}

you'll have:

public class CountriesController : BaseController
{
    private readonly IRepository _repository;

    public CountriesController(IRepository repository)
    {
        _repository = repository;
    }
}

It might be a good idea to ask another question about your unit-of-work and repository that you use here.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more specific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post more code but its design doesn't look right.

It's difficult to come up with an example. The only thing I would change about your code in the first place would be to use the repository instead of the invalid unit-of-work. So instead:

public class CountriesController : BaseController
{
  private IUnitOfWork unitOfWork;

  public CountriesController(iMSTUnitOfWork _unitOfWork)
  {
      unitOfWork = _unitOfWork;
  }
}

you'll have:

public class CountriesController : BaseController
{
    private readonly IRepository _repository;

    public CountriesController(IRepository repository)
    {
        _repository = repository;
    }
}

It might be a good idea to ask another question about your unit-of-work and repository that you use here.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.

added 793 characters in body
Source Link
t3chb0t
  • 44.7k
  • 9
  • 84
  • 191

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more spcific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post mremore code but its design doesn't look right.

It's difficult to come up with an example. The only think I would change about your code in the first place would be to use the repository instead of the invalid unit-of-work. So instead:

public class CountriesController : BaseController
{
  private IUnitOfWork unitOfWork;

  public CountriesController(iMSTUnitOfWork _unitOfWork)
  {
      unitOfWork = _unitOfWork;
  }
}

you'll have:

public class CountriesController : BaseController
{
    private readonly IRepository _repository;

    public CountriesController(IRepository repository)
    {
        _repository = repository;
    }
}

It might be a good idea to ask another question about your unit-of-work and repository that you use here.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more spcific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post mre code but its design doesn't look right.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more spcific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post more code but its design doesn't look right.

It's difficult to come up with an example. The only think I would change about your code in the first place would be to use the repository instead of the invalid unit-of-work. So instead:

public class CountriesController : BaseController
{
  private IUnitOfWork unitOfWork;

  public CountriesController(iMSTUnitOfWork _unitOfWork)
  {
      unitOfWork = _unitOfWork;
  }
}

you'll have:

public class CountriesController : BaseController
{
    private readonly IRepository _repository;

    public CountriesController(IRepository repository)
    {
        _repository = repository;
    }
}

It might be a good idea to ask another question about your unit-of-work and repository that you use here.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.

Source Link
t3chb0t
  • 44.7k
  • 9
  • 84
  • 191

Unit of Work

What strikes me the most in you design is the invalid usage of the Unit of Work. You use it only to get the repository via its property but

a Unit of Work keeps track of everything you do during a business transaction that can affect the database. When you're done, it figures out everything that needs to be done to alter the database as a result of your work.

Like an Entity in Entity Framework that tracks changes.

You might as well pass via DI the repository directly and it wouldn't change anything.

Naming a Unit of Work a unit-of-work is also a bad practice. It's just a placeholder for a more spcific type in your domain. Maybe a Country or something but definitely not just a unit-of-work.

I cannot say anything more then this because you didn't post mre code but its design doesn't look right.

MVC

The first design looks OK. I cannot say anything about the second one, it's too short and it doesn't interact with the model - perhaps it doesn't have to. I don't know.

Minor issues

Using regions like you use them is worse then not using them at all. I had to remove them first so that your code becomes readable. Personally I don't like them. Furthermore your code formatting is very strange with those multiple line breaks which makes it hard to read too.