I could have a VegetableManager (or
something) class where the load method
is, but does this actually mean less
repeated code? Then in each screen I
have to create object of
VegetableManager and call its load
method anyway.
The point of doing this is not efficiency (i.e. performance). The point is to encapsulate the details of loading that data into a single isolated object. Say for example that your site gets really big and you decide to move the data storage to a database for scalability and performance. In the existing code as you described, you'll have to go through each user control or page and change the logic of the load method. At the best this is a pain, and at the worst you miss some or copy-paste incorrectly. If the logic is encapsulated into a dedicated object, whose only responsibility is to know how to load the data from somewhere, then you only have to make the change once.
codebehind of user control:
protected void Page_Load(object sender, EventArgs e) {
var veggieManager = new VegetableManager();
VeggieListControl.DataSource = veggieManager.GetAll();
VeggieListControl.DataBind();
}
VegetableManager.cs:
public class VegetableManager {
private static Collection<Vegetable> _veggies;
private static object _veggieLock;
public ReadOnlyCollection<Vegetable> GetAll() {
if (_veggies == null) {
lock(_veggieLock) { //synchronize access to shared data
if (_veggies == null) { // double-checked lock
// logic to load the data into _veggies
}
}
}
return new ReadOnlyCollection(_veggies);
}
public void Add(Vegetable veggie) {
GetAll(); // call this to ensure that the data is loaded into _veggies
lock(_veggieLock) { //synchronize access to shared data
_veggies.Add(veggie);
// logic to write out the updated list of _veggies to the file
}
}
}
Because _veggies
is static
, there is only one collection of veggies in memory, despite the fact that multiple callers will instantiate VegetableManager
. But because it's static, if you have a multi-threaded application (e.g. a website) you must synchronize access to that field across all threads (hence the lock
s).
This is the tip of the iceberg in terms of good object-orientation. I recommend perusing UncleBob's SOLID principles, and Domain-Driven Design (free e-book).
So, yes you are repeating something, but all you're repeating is a method call, and that is ok to repeat. DRY means to mitigate the duplication of "logical" code, i.e. decision-making and algorithms; simple method calls do not fall under this. However, if you want, you can consolidate logic into a base class do this, effectively isolating the user controls from having to know about VegetableManager, though I think this is object-orientation overkill, or OOO :-)
public abstract class FoodUserControl : UserControl {
protected List<Vegetable> GetVeggies() {
return new VegetableManager().GetAll();
}
}
Then your actual controls would derive from this instead of from UserControl.
Update
Eager-loading VegetableManager.cs:
public class VegetableManager {
private static Collection<Vegetable> _veggies;
private static object _veggieLock;
static VegetableManager() {
// logic to load veggies from file
}
public ReadOnlyCollection<Vegetable> GetAll() {
return new ReadOnlyCollection(_veggies);
}
public void Add(Vegetable veggie) {
lock(_veggieLock) { //synchronize access to shared data
_veggies.Add(veggie);
// logic to write out the updated list of _veggies to the file
}
}
}
Notice this eager-loading version doesn't have to do double-checked locking around the load code in the constructor. Also notice that the load code is in a static
constructor, since this code initializes a static
field (otherwise, you'd be reloading the data from the file on every construction into the same shared static
field). Because veggies are eager-loaded, you don't need to load in GetAll or Add.