Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

When I need some item, should I just use its "int id" instead?

Tags:

c#

oop

My application has InstrumentFactory - the only place where I create Instrument instance. Each instrument instance contains several fields, such as Ticker=MSFT and GateId=1 and also unique Id =1.

And now I realized that I almost never need Instrument instance. In 90% of cases I just need Id. For example now I have such method:

public InstrumentInfo GetInstrumentInfo(Instrument instrument)
{
    return instrumentInfos[instrument.Id];
}

We know that we should not pass in parameters more information than required. So this code probably should be refactored to:

public InstrumentInfo GetInstrumentInfo(int instrumentId)
{
    return instrumentInfos[instrumentId];
}

90% of my code can now be refactored to use instrumentId instead of Instrument.

Should I do that? Changing everywhere Instrument to instrumentId will make it as a hard requirement (each Instrument should have exactly one unique id). But what benefits will I have? In return of "hard requirements" I want to have some benefits for that... (speed, readability?) But I don't see them.

like image 891
Oleg Vazhnev Avatar asked Aug 12 '12 17:08

Oleg Vazhnev


2 Answers

Using ids everywhere instead of the object is wrong approach and it goes against the spirit of OOP.

There are two big advantages to using the object itself:

  1. It's type-safe. You can't accidentally pass something like Person to the first version, but you can accidentally pass person.Id to the second.
  2. It makes your code easy to modify. If, in the future, you decide that you need long ids, or some other way to identify a unique Instrument, you won't need to change the calling code.

And you should probably change your dictionary too, it should be something like Dictionary<Instrument, InstrumentInfo>, not Dictionary<int, InstrumentInfo>, like you have now. This way, you get both of the advantages there too. To make it work, you need to implement equality in Instrument, which means properly overriding Equals() and GetHashCode() and ideally also implementing IEquatable<Instrument>.

like image 103
svick Avatar answered Nov 10 '22 05:11

svick


It's always better to work in terms of objects than primitive values like integers. If tomorrow your requirements happen to change and you need more than just the ID, it is easy to add those to the Instrument object instead of changing all your code.

like image 25
casablanca Avatar answered Nov 10 '22 03:11

casablanca