It will probably be a long post but please bear with me. The basic idea is this:
public int InsertPersonAndGetPersonId(Person person){
_dbContext.Insert(person);
return person.PersonId;
}
The method above is simple but it violates clean programming principles.
- It is against Command/Query separation in methods.
- It does more than one job.
When I am evaluating different approaches, I usually list the pros and cons and choose the one that has the least cons and the least trade-offs. Therefore, honestly, the method above looks better than the alternatives listed below. But I still would like to get the opinions of SO community and perhaps I learn a new pattern that works best with this challenge.
Alternative #1
The one alternative is to have two methods. While the one is inserting new records, the other gets the lastly added personId
from database. But this is not reliable unless you prevent database from accepting new person insertion between the time you insert a record and get its id from database.
You can even do filtering by the a property of Person
(Name
for instance) when getting the record from database but in addition to what I said above, there can also be more than one person who have the same name but different PersonId
s.
It is also doing one more database trip. I am a pragmatic person and don't like to speculate about performance without actually measuring it. However, if I can prevent something with a simple change which will contribute to the performance even slightly, then I find it silly not to do it. Of course, while doing it, I also consider clean code practices.
Alternative #2
The other method can be to change InsertPersonAndGetPersonId
is in and have something like following:
public class PersonRepository
{
private int _personId;
public void InsertPerson(Person person){
_dbContext.Insert(person);
_personId = person.PersonId;
}
public int GetLastPersonId
return _personId;
}
}
Even though I don't like the name of this method GetLastPersonId()
, which may bring a different personId
than expected but let's assume that it returns the id
of person
object. The reason it is bad, besides what I already said, it is modifying the state of the object, therefore have a side effect.
Alternative #3
We can simply have the method below:
public void InsertPerson(Person person){
_dbContext.Insert(person);
_personId = person.PersonId;
}
and since person
is reference type, we can access person.PersonId
like following:
var personRepository = new PersonRepository();
var person = new Person() {Name="Hello"};
personRepository.InsertPerson(person);
Console.WriteLine(person.PersonId);
Did I like it? No! It is way too hidden and unpredictable and you don't really know it unless you check the implementation detail and then we break the beauty of abstraction.
Alternative #4
We can use out
like follows:
public void InsertPerson(Person person, out int personId){
_dbContext.Insert(person);
personId = person.PersonId;
}
But this looks more silly and cumbersome than the first one InsertPersonAndGetPersonId
. If I am going to have to return something, then I would return it using return
and make the signature more explicit for the developers. Also in fact, out
and ref
makes more sense when we need to return multiple values. For instance TryParse()
, returns boolean
but also you can get the parsed value using out
or ref
too.
Update
Due to a couple of comments, I decided to clarify my question a little bit more. What I am asking is how to get the PersonId
without breaking the clean code principles. I am using EF and therefore getting the id from database is not a problem and in fact you can see it in my first example. Sorry for the confusion.
Aucun commentaire:
Enregistrer un commentaire