0

Ran across the C# code below (piece 1) - reduced for clarity, it also had instance methods, many more members, etc. Am I missing some very smart programming pattern here or it can be reduced to piece 2 without hassle? The Address() type is very simple, more a data transfer object really.

Piece 1:

public class MyStuff
{
    private IAddress _address;

    public MyStuff()
    {
            SetAddress(_address = new Address());
    }

    private void SetAddress(IAddress addr)
    {
        _address = addr;
    }
}

Piece 2:

public class MyStuff
{
    private IAddress _address = new Address();

    public MyStuff()
    {
        //The constructor is probably redundant too
    }
}
demp
  • 12,665
  • 2
  • 23
  • 17
  • I'm guessing piece one is designed to be loosely coupled for DI? Were there any other constructors? – Dave Hogan Jul 09 '12 at 14:49
  • I suppose _addressDao is also a private member? – Christoffer Jul 09 '12 at 14:50
  • Are you sure you reduced it for simplicity correctly? It looks like someone is trying to use Dependency Injection. There's no public constructor that looks like this: public MyStuff(IAddress address)...?? – aquinas Jul 09 '12 at 14:51
  • 3
    @demp You need to clarify what `_addressDao` is as it completely changes the answer. – Adam Houldsworth Jul 09 '12 at 14:56
  • It would be easier to explain the differences (or similarities) of the two classes if we had the full code. From the provided code, piece 2 will not function the same as Piece 1 was intended to. _addressDao is never set in Piece 2, and there is no way of telling if SetAddress is called anywhere else in the class. – Tom Jul 09 '12 at 14:57

2 Answers2

2

You have not clarified _addressDao so I will cover both bases:


The following assumes that _addressDao exists somewhere and needs to remain.

You cannot compress this into piece 2 because of _addressDao. Piece 1 is setting _address inline during the method call then also setting _addressDao, your second piece omits _addressDao, so is not equivalent.


The following assumes that _addressDao is a typo and/or no longer needs to remain.

Piece 2 can be a valid alternative to piece 1 if we assume _addressDao is not a concern. It might not be the best approach for your situation, but you are not asking that so I won't go into a diatribe about it.


In-line class member initialization, if memory serves, compiles into the same stuff as if you were initializing it directly in the constructor anyway. The end result is the same behaviour, just different looking code.

In piece 2 you are also correct that the public parameterless constructor is no longer needed, in the absence of any constructors, this is added by default.


With regards to potential design patterns being used here, it could be an attempt at IoC or DI - but it appears to be successful at neither of them.
Adam Houldsworth
  • 63,413
  • 11
  • 150
  • 187
  • 1
    I'm under the impression `_addressDao` is a typo for just `_address`. My guess. – LarsTech Jul 09 '12 at 14:58
  • @LarsTech So am I, but I don't like assumptions. I also cover both situations anyway so it's moot. – Adam Houldsworth Jul 09 '12 at 15:00
  • Adam is right on target. _address is set in both pieces. However, _addressDao is only set in Piece 1 and therefore it is not equivalent to piece 2. If _addressDao is a typo, and you meant _address. Then they are equivalent, only Piece 1 sets _address twice (to the same value so there is no difference). – Tom Jul 09 '12 at 15:15
  • Adam, you were right, "Dao" was my misspell. I edited the question to reflect that. And yes, to me it also looked like _address was initialized twice in piece 1. However, I wasn't sure if I missed anything else - hence the question. IoC or DI - neither are needed here. – demp Jul 09 '12 at 16:00
1

Ideally they will do the same thing. What actually happens at Piece 2 at runtime is the initialization of _address gets run in the MyStuff() constructor, before your code or any this()/base() calls.

Good info here: C# member variable initialization; best practice?

I think Piece 2 is easier to read, but that's purely personal preference.

That being said, your first piece is assigning _address twice. Once when you call into the function, and once into the function itself.

Community
  • 1
  • 1
Stevoman
  • 976
  • 9
  • 25