I believe you're misunderstanding SRP, or at least are only applying it partially.
- First solution: In the
KPI class
It seems like it respects the SRP, the KPI class is responsible for the values of the KPI.
There are two responsibilities here: retrieving the value, and storing the value (in memory).
"It's related to the KPI" is not an accurate description of a reponsibility.
Responsibilities are generally expressed as verbs: storing the data, logging the messages, printing the output, sorting the list, formatting the date, ...
- Second solution: In the
Item class
My concern is that it breaks SRP, the Item should not be in charge of setting the values, should it?
I have to agree with you here, Item is definitely not where you should put it. But your reasoning is somewhat flawed (in the same way as the previous bullet point)
What would be the best way of doing it? Is there a third, better way I did not think about?
In this case, performance is not really an issue, the table and the tree are small, but I am learning, and would like to learn about the best practices.
In the interest of learning, I'll keep the answer demonstrative but not implemented to an expert professional level.
You also didn't specify how you want to fill in the data, and I'd like my answer to focus on the intention of the code, rather than the implementation. So I'm going to mainly focus on structure and outset, rather than showing you specific code.
First of all
A minor philosophical reframing.
You've left a few gaps in your question, e.g. how the items are connected or what the main purpose of the system is. I'm filling in some blanks here based on experience and inference.
As I understand it, Item and KPI are two parts of the same entity. KPI is the actual meat and potatoes, the value of the object. Item, on the other hand, is merely a wrapper which helps you connect KPI values to each other (in a hierarchical structure).
Unless you've omitted an important part (or I'm glossing over it), I see no reason for Item and KPI to be two separate classes. For the rest of the answer (unless you can explain why this is not correct), I'm going to merge these into a single class:
public class KpiItem
{
public string Name { get; set; }
public double Value { get; set; }
public List<KpiItem> Children { get; set; }
// Personally, I prefer also having a link to the parent.
// But this is optional.
public KpiItem Parent { get; set; }
}
Secondly, based on your table data, there is no explanation how the hierarchy of items is stored. Maybe this is stored in another table (e.g. Items) which you did not show us. Maybe you simply didn't get to developing that part yet.
Whatever the reason, we need to know how these are linked before we can retrieve them with the correct hierarchy. So I'm going to add a column to your table: the name of the parent.
+------+-------+-------------------+
+ Name + Value + Parent_Name +
+------+-------+-------------------+
+ foo + 5 + (null) +
+ bar + 4.56 + foo +
+------+-------+-------------------+
In other words: bar is a child of foo. foo is no one's child, it's a top-level element.
Third solution: A separate class made for handling data retrieval.
This is a much more SRP-friendly way to approach the problem.
Now, we haven't quite established what you want to retrieve:
- A single element (by name)
- A single element (by name) plus its direct children
- A single element (by name) plus all of its descendants (children, grandchildren, greatgrandchildren, ...)
For demonstrative purposes, I'll show a way to retrieve a single element (by name) plus its direct children. If you want to get all descendants, it's a simple matter of implementing recursion (unless your dataset can have circular references).
Since you're using SQL queries, so will I. We will need two queries:
- Retrieving an item based on its name.
- Retrieving an item based on its parent's name.
The queries are relatively simple:
SELECT * FROM KPIITEMS WHERE NAME = 'foo'
SELECT * FROM KPIITEMS WHERE PARENT_NAME = 'foo'
Sidenote
Notice that I'm doing SELECT * instead of SELECT value. For your current example, you already know the name you're looking for, so you're only interested in the value.
But this quickly falls apart when you start looking for other things, e.g. the first alphabetical item, the item with the longest name, ... Because now, you don't know the name.
Generally, you'll want to create your KpiItem based on the returned query; not based on the filters you supplied. While one could logically explain the other; this only applies in rare cases and is not a good general approach. This is why I did SELECT *, so that the query returns all columns so we can fully populate the KpiItem.
Since the question doesn't focus on how to retrieve data from SQL, I'm going to omit it for simplicity's sake. Let's assume these methods exist:
public KpiItem GetItem(string query);
public List<KpiItem> GetItems(string query);
In other words, if we supply the correct query to this method, it gives us the needed objects.
An example implementation of this third class would be as follows:
public class DataFetcher
{
public KpiItem GetKpiItemWithChildren(string name)
{
var theItem = GetKpiItem_ByName(name);
theItem.Children = GetKpiItemsBy_ParentName(name);
return theItem;
}
private KpiItem GetKpiItem_ByName(string name)
{
return GetItem("SELECT * FROM KPIITEMS WHERE NAME = '" + name + "'");
}
private List<KpiItem> GetKpiItems_ByParentName(string parentName)
{
return GetItems("SELECT * FROM KPIITEMS WHERE PARENT_NAME = '" + parentName + "'");
}
}
And then you can use this class as follows:
var fetcher = new DataFetcher();
var fooItem = fetcher.GetKpiItemWithChildren("foo");
Sidenote
There are things missing for this example to be a properly usable (professional) piece of code. Most importantly, this does not protect against SQL injection.
You really should protect against SQL injection (read up on it when you're ready), but I've omitted it from the current example because (a) as a beginner you're likely not at that point yet and (b) I'm trying to keep the example as simple as possible so you can understand the intended structure.
Your example case is simple. Once you get into building applications that have many tables and considerably more complex data structures, this answer doesn't really apply anymore. But I'm inferring that you're currently looking for a beginner's explanation, not so much an expert-level implementation.
If you're interested in scaling upwards, a good place to start would be by reading up on Entity Framework and LINQ. But there's no need to rush, there is merit to focusing on the basics first (as they say, only the fools rush in).
