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(); }
Improve this question
3
  • 2
    I would go with second one. I dislike passing list into method and the method adding to it. Commented Feb 1, 2018 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. Commented Feb 1, 2018 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 ;) Commented Feb 2, 2018 at 23:05

3 Answers 3

Reset to default
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.

Improve this answer
0
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.

Improve this answer
7
  • 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 Commented Feb 1, 2018 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. Commented Feb 1, 2018 at 14:30
  • @BerinLoritsch, That is correct. Commented Feb 1, 2018 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 Commented Feb 1, 2018 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). Commented Feb 1, 2018 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.

Improve this answer
4
  • 1
    I wonder what the reasons for the down votes were? Commented Feb 1, 2018 at 15:12
  • I do not know, I do not have the power to vote because of insufficient reputation because i'm new user Commented Feb 1, 2018 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. Commented Feb 1, 2018 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). Commented Feb 2, 2018 at 8:35

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.