How can I optimize this code?

int getID<T>(T senderClass, int ID) { switch (typeof(T).Name) { case "Counter"/* - имя класса*/: for (int i = 0; i < countersArray.Count /*list экземпляров класса*/; i++) { if (countersArray[i].ID != ID) continue; return i; } break; case "Organization": for (int i = 0; i < organizationsArray.Count; i++) { if (organizationsArray[i].ID != ID) continue; return i; } break; case "Representative": for (int i = 0; i < representativesArray.Count; i++) { if (representativesArray[i].ID != ID) continue; return i; } break; case "Service": for (int i = 0; i < servicesArray.Count; i++) { if (servicesArray[i].ID != ID) continue; return i; } break; case "Consumer": for (int i = 0; i < consumersArray.Count; i++) { if (consumersArray[i].ID != ID) continue; return i; } break; case "ServiceByContract": for (int i = 0; i < servicesByContract.Count; i++) { if (servicesByContract[i].ID != ID) continue; return i; } break; case "Contract": for (int i = 0; i < contractsArray.Count; i++) { if (contractsArray[i].ID != ID) continue; return i; } break; case "Flat": for (int i = 0; i < flatsArray.Count; i++) { if (flatsArray[i].ID != ID) continue; return i; } break; case "User": for (int i = 0; i < usersArray.Count; i++) { if (usersArray[i].ID != ID) continue; return i; } break; } return 0; }
  • countersArray - these collections in conditions, they are of different types? - tym32167 pm
  • and what is senderClass ? - tym32167
  • There is a class Counter, countersArray - List <Counter> countersArray = new List <Counter> (), everything else by analogy, I use senderClass to determine the type - user307344
  • one
    show also how you use your method in your code, what is it for? - tym32167 pm
  • one
    But in general, I asked for a sample code, because most likely you are doing something wrong, since you generally need a similar construction like yours - tym32167

2 answers 2

You can delete your method and replace it with calls to Array.FindIndex . For example:

 var data = Enumerable.Range(0, 10).Select(x => new MyClass() {ID = x}).ToArray(); var ind = Array.FindIndex(data, item=>item.ID == 3); Console.WriteLine(ind);

Expected output

 3

In case you work with a list, not an array, the difference is small

 var data = Enumerable.Range(0, 10).Select(x => new { ID = x }).ToList(); var ind = data.FindIndex(item => item.ID == 3); Console.WriteLine(ind);

Expected output

 3
  • judging by the comments there List<> , so you need to take FindIndex from him :) - Grundy
  • @Grundy thanks, added - tym32167

Judging by the use of the Count property, all your xxxxArray is actually a List<T> , so you can use its own FindIndex method to search for the index.

If your task is simply to reduce the amount of code, then you can use the capabilities of the switch construction from C # 7.

In addition, judging by the use of list items, your senderClass itself contains an ID and you can get rid of the additional ID parameter in the method.

As already written @ tym32167 , when the element is not found, using 0 as a result is not correct, because 0 is a valid index of any array, except for an empty one, therefore such an answer will lead to errors in the future. It is better to use a value that can never be a valid index. -1 just such a value is standard for all search methods in .NET, when exactly the index of the element is searched.

All the above described will look like this:

 int getID<T>(T senderClass) { switch (senderClass) { case Counter counter: return countersArray.FindIndex(item => item.ID == counter.ID); case Organization organization: return organizationsArray.FindIndex(item => item.ID == organization.ID); case Representative representative: return representativesArray.FindIndex(item => item.ID == representative.ID); case Service service: return servicesArray.FindIndex(item => item.ID == service.ID); case Consumer consumer: return consumersArray.FindIndex(item => item.ID == consumer.ID); case ServiceByContract serviceByContract: return servicesByContract.FindIndex(item => item.ID == serviceByContract.ID); case Contract contract: return contractsArray.FindIndex(item => item.ID == contract.ID); case Flat flat: return flatsArray.FindIndex(item => item.ID == flat.ID); case User user: return usersArray.FindIndex(item => item.ID == user.ID); default: return -1; } }

In general, the construction itself is rather strange and suggests a design error, but without being able to see where and how it is used, it is almost impossible to recommend anything.

Immediately we can note the problem of this method. Since there is no restriction on the type-parameter T , it is impossible to understand why an element was not found: because the element is not in the required list or because the method was applied to an incorrect type (called with an incorrect type parameter). This can be corrected by returning some other negative value in the default section, for example -2 , which will add information, but a simple error: the entity was added, and the method was not corrected - it would still lead to difficulties in diagnostics and debugging.

It is also necessary to allocate a common interface for all entities, at least something of the type is clearly suggested:

 interfase IHaveID { int ID { get; set: } }

Since all your entities have an ID. For other interfaces I can not say anything, not enough information.

On the rights of telepathic%):

Probably the class to which this method belongs is used as a version of the single repository. If this is true, then it is worth dividing it into separate repositories for each type of entity, naturally selecting and describing identical functionality by interfaces.