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

Accuses Readonly for a property that is not ReadOnly #113

Closed
Ridermansb opened this issue Jun 4, 2013 · 4 comments
Closed

Accuses Readonly for a property that is not ReadOnly #113

Ridermansb opened this issue Jun 4, 2013 · 4 comments
Labels

Comments

@Ridermansb
Copy link

In my domain layer have the class:

public class Telefone : IEntity
{
    public virtual int Id { get; protected internal set; }
    public virtual string Tipo { get; set; }
    public virtual string Numero { get; set; }
}

The Id is protected internal. This is deliberate, who generates the Id is the database and should be ReadOnly.
But in the test layer, must have access to the setter of this property.

So in my domain layer added the attribute in AssemblyInfo.cs:

[assembly: InternalsVisibleTo("MyTestLayer")]

This will allow me to set the Id in tests layer but not in the domain layer!

In test layer
new Telefone().Id = 1; // Works!!

In UI layer
new Telefone().Id = 1; // Not compile! Not works!

Problem

var pessoa = fix.Build<Pessoa>()
    .With(p => p.Nome)
    .Do((pess) =>
    {
        fix.Build<Telefone>()
            .With(p => p.Id)
            .With(p => p.Tipo)
            .With(p => p.Numero)
            .OmitAutoProperties()
            .CreateMany(10).ToList().ForEach(pess.Telefones.Add);

        fix.Build<Email>()
            .With(p => p.Id)
            .With(p => p.Tipo)
            .With(p => p.Endereco)
            .OmitAutoProperties()
            .CreateMany(10).ToList().ForEach(pess.Emails.Add);
    })
    .OmitAutoProperties()
    .Create();

Theoretically, the above code should work, but when my road test an error occurs:

The property "Id" is read-only.

All properties Ids are protected internal.

In the same file, same test can do something like:

new Telefone().Id = 1; // Works!!
new Email().Id = 1; // Works!!

@ploeh
Copy link
Member

ploeh commented Jun 4, 2013

This clearly tells you that you have a leaky abstraction in your Domain Model. A Domain Model should not be designed with any particular boundary technology in mind.

This particular issue is very clearly telling you that your Domain Model is not reusable.

InternalsVisibleTo has no effect because it only allows access to your test library, but AutoFixture is a different assembly entirely. This just tells you that no other client will ever be able to reuse your Domain Model.

The best course of action is to reconsider your API design. Normally, I'd do something like this:

public class Telefone : IEntity
{
    public Telefone(int id)
    {
        this.Id = id;
    }

    public virtual int Id { get; private set; }
    public virtual string Tipo { get; set; }
    public virtual string Numero { get; set; }
}

@Ridermansb
Copy link
Author

This generated a lot of discussion in a group of architecture that I follow.
There is a major problem with this approach:

  1. Who is responsible for generating the Id is the database, it is only generated when the object is saved.
  2. When I create a new object, which Id must pass in the constructor? I can not put the Id in the constructor.
  3. The Id is a int sequential. The repository layer is the closest layer of the Domain. To generate the Id would do something like max (id) + 1 and then set the Id of my object. But how to set the Id if it is readonly?
  4. The only way to set the Id of in a ReadOnly property is within the class itself or using a proxy/reflection

@ploeh
Copy link
Member

ploeh commented Jun 4, 2013

Agreed that these are all problems. All four of them strongly indicate that the Domain Model depends on the persistence layer, and can't be made to work without it.

  1. Why should the database assign IDs? The only requirement is that an ID is unique. This is why we have GUIDs.
  2. Pass a new Guid instance to the constructor.
  3. Why is the ID sequential? There's rarely any architectural reason for IDs to be sequential.
  4. Agreed. There's a reason access modifiers exist.

@Ridermansb
Copy link
Author

I understand the problem. I can not change the PKs type to Guid, would really work.

I understand now that the problem is not related to AutoFixture, but if you're interested. I create a sample project to discuss this.

thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants