Does the class have unnecessary and violating Single responsibility principle method?

Question

I encountered with the following code in the work. The MyItemCoordinator should receive MyItem objects, process them, add them to MyItemList collection and then pass them to the listeners.

My questions are:

1. When I saw NewMyItem method I thought that MyItemCoordinator is a factory but when I saw NewMyItem's implementation I found out that it creates, sorts, calling to listeners and it's not static. So MyItemCoordinator is not a factory and NewMyItem method does way more things that it should to do according to its name, right?

2. If it's not a factory so why NewMyItem method receives so many params, creates an object, adds to the collection? Why not to do:

   example:

   var coordinator = new MyItemCoordinator();
   var myItem = new MyItem(...PARAMS..)
   coordinator.AddMyItemToList(myItem)
   

   Inside AddMyItemToList we can sort and call the listeners. This is more clear way to do, isn't it?

3. So, is this implementation correct? Doesn't it have something wrong?

   public class MyItemCoordinator
   {    
       ...
   
       public IList<MyItem> MyItemList = new List<MyItem>();
   
       public void NewMyItem(int id, ...15_MORE_PARAMS)
       {
           MyItem myItem = new MyItem(id,...ONLY_5_PARAMS_WITHIN_15_ARE_USED);
           AddMyItemToList(myItem);
           SortMyItemList();
           if (MyItemDetectedHandler != null)
               MyItemDetectedHandler(this);
       }
   
       public void AddMyItemToList(MyItem myItem)
       {
           //makes some checking and adds myItem to MyItemList
       }
   
       ...
   }
   

Answer 1

You're right, but you should take this to its logical conclusion.

• Creating item belongs to a factory class.
• Tracking a list of items (coordinating?) is another responsibility.
• Handling the new item (or list? Not clear from the code) belongs to a third class.

With this design you can reliability test each component in isolation. Merging these responsibilities into a single class makes testing difficult and blocks alternate implementations.