0

I have a small question. Which approach is correct in the context of SOLID principles? 1 or 2 ?

In the first case, the "CreateTask" method does not return the Task object, but places it on the list that it accepts as the method argument.

In the second case, the "CreateTask" method does not accept arguments, but returns a task object that must be placed in the Tasks list

1.

var tasks = new List<Task>();
CreateTask1(tasks);
CreateTask2(tasks);
CreateTask3(tasks);

private void CreateTask1(List<Task> tasks)
{
   // some other logic about new task object       
   tasks.Add(new Task());
}

private void CreateTask2(List<Task> tasks)
{
   // some other logic about new task object       
   tasks.Add(new Task());
}

private void CreateTask3(List<Task> tasks)
{
   // some other logic about new task object       
   tasks.Add(new Task());
}

2.

var tasks = new List<Task>();
tasks.Add(CreateTask1());
tasks.Add(CreateTask2());
tasks.Add(CreateTask3());

private Task CreateTask1()
{
   // some other logic about new task object
   return new Task();
}

private Task CreateTask2()
{
   // some other logic about new task object
   return new Task();
}

private Task CreateTask3()
{
   // some other logic about new task object
   return new Task();
}
  • 2
    I would go with second one. I dislike passing list into method and the method adding to it. – Euphoric Feb 1 '18 at 13:41
  • SOLID is not a universal law (like Gravity). Its interpretation depends on the context. For us to say which approach is correct, we need more context. Real code if possible. Otherwise is a guesswork. – Laiv Feb 1 '18 at 21:37
  • @Laiv, that's not a good analogy. Newtons "universal law of gravity" is a bit of a rubbish theory to be honest. It only really works for low speeds and low mass. It doesn't eg handle light lensing at all. So maybe SOLID doesn't want to aspire to be a universal law ;) – David Arno Feb 2 '18 at 23:05
2

Though SOLID argues for #2, you should also do some thinking on your own about possible future changes.

What if you don't want to use a List? Say, you want to use a SuperSpecialList or a Map?

What if you need to add it to two lists?

What if that List is a Queue with a maximum length and it is full?

All of these also argue for design #2.

3

I'd go for option 3:

var tasks = new List<Task>
{
    CreateTask1(),
    CreateTask2(),
    CreateTask3()
};

However, I'm unclear how the question relates to SOLID principles

Update

With regards to the single responsibility principle, option 1 is "is responsible for creating the object and inserting it into the list". Does this mean it breaks the principle? No. It no more breaks it than option two with its "responsible for creating the object and returning it back". Rephrase these as "insert a created object in the list" or "return a created object" and the "and" goes away. The presence of "and" in a description of a method's functionality is an unreliable way of assessing whether it has one responsibility.

Option 1 is ugly as it involves mutating a parameter. Option 2 is better, but doesn't make proper use of language features. So go with option 3.

  • In the first case, the "CreateTask" method is responsible for creating the object and inserting it into the list - does it not violate the principle of single responsibility? In the second case, the "CreateTask" method is responsible only for creating the object and returning it back – Crazy Man Feb 1 '18 at 13:47
  • 2
    @DavidArno, your solution is functionally equivalent to Option 2. There is no change to the task creators so they are responsible for creating the object and returning it back. It's just a compiler feature that adds the items in the original list for you. – Berin Loritsch Feb 1 '18 at 14:30
  • @BerinLoritsch, That is correct. – David Arno Feb 1 '18 at 14:33
  • @CrazyMan: The Singe Responsibility Principle is not about the responsibilities of single methods, so it is irrelevant here. See en.wikipedia.org/wiki/Single_responsibility_principle – JacquesB Feb 1 '18 at 14:50
  • @CrazyMan: That's not entirely true. SRP is about avoiding coupling between code that changes for different reasons, by separating such code into different organizational units, so you can choose to apply it at the function level to make the code more maintainable (and you also have to decide to what extent you want to apply it). – Filip Milovanović Feb 1 '18 at 15:32
1

The SOLID principles are principles for designing object-oriented software. Your code example is not object oriented - it consists of stand-alone functions and some top-level code which is not even in a function. To interpret the code according to SOLID we need to know which class performs these operations for what purpose, and what the underlying responsibilities are. Depending on these factors, either example could be best.

Note specifically that the Single Responsibility Principle is not concerned about the responsibilities of individual methods, but apply on the class or module level. So it is not applicable on these code examples.

  • 1
    I wonder what the reasons for the down votes were? – PeterSW Feb 1 '18 at 15:12
  • I do not know, I do not have the power to vote because of insufficient reputation because i'm new user – Crazy Man Feb 1 '18 at 16:50
  • @PeterSW: Perhaps people think SOLID just means "whatever code I prefer", and does not know it is an acronym of some specific principles. – JacquesB Feb 1 '18 at 17:07
  • 1
    I downvoted due to the incorrect statement, "The SOLID principles are principles for designing object-oriented software". The SOLID acronym has focused on OO, but the underlying principles (minimising coupling, maximising cohesion, designing to interfaces and the LSP) all have a far broader reach than just OO). – David Arno Feb 2 '18 at 8:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.