How can I simplify and optimize this C# code

Question

I have a method like this :

public ConcurrentBag<FileModel> GetExceptionFiles(List<string> foldersPath, List<string> someList)
{
    for (var i = 0; i < foldersPath.Count; i++)
    {
        var index = i;

        new Thread(delegate()
        {
            foreach (var file in BrowseFiles(foldersPath[index]))
            {
                if (file.Name.Contains(someList[0]) || file.Name.Contains(someList[1]))
                {
                    using (var fileStream = File.Open(file.Path, FileMode.Open))
                    using (var bufferedStream = new BufferedStream(fileStream))
                    using (var streamReader = new StreamReader(bufferedStream))
                    ...

To give you more details:

This methods starts n threads (= foldersPath.Count) and each thread is going to read all the files which contains the strings listed in someList.

Right now my list contains only 2 strings (conditions), this is why im doing :

file.Name.Contains(someList[0]) || file.Name.Contains(someList[1])

What I want to do now is to replace this line with something that check all elements in the list someList

How can I do that?

Edit

Now that I replaced that line by if (someList.Any(item => file.Name.Contains(item)))

The next question is how can I optimize the performance of this code, knowing that each item in foldersPath is a separate hard drive in my network (which is always not more that 5 hard drives).

Are you sure these threads actually accelerate the whole process? You should be I/O bound anyway. — Lucas Trzesniewski
– Lucas Trzesniewski, Commented Nov 26, 2014 at 13:08
HOLY £$%& you are creating a ton of threads, and not even using the thread pool. First up. YOU SHOULD NOT BE THREADING with a modern computer programming language like C# for something like this (Java, go nuts). Use async await. — Aron
– Aron, Commented Nov 26, 2014 at 13:09
In your question text, you ask for simplification of the code, while in your tags, you list optimization and performance. Those might be contradictory goals to aim for. — O. R. Mapper
– O. R. Mapper, Commented Nov 26, 2014 at 13:10
@LucasTrzesniewski, I think (Yes), because each folder in foldersPath is actualy a separte hard drive — Farmer
– Farmer, Commented Nov 26, 2014 at 13:10
Should this not rather be asked on codereview.stackexchange.com ? — Bernd Linde
– Bernd Linde, Commented Nov 26, 2014 at 13:12

Joe · Accepted Answer · 2014-11-26 13:08:08Z

3

You could use something like if (someList.Any(item => file.Name.Contains(item)))

This will iterate each item in someList, and check if any of the items are contained in the file name, returning a boolean value to indicate whether any matches were found or not

answered Nov 26, 2014 at 13:08

Joe

1,2143 gold badges15 silver badges34 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Dirk Over a year ago

It should be Any instead of All.

Joe Over a year ago

Spotted that while I was editing it to add a little more of an explanation. Thanks @Dirk

Aron · Accepted Answer · 2014-11-26 13:49:26Z

Fristly.

There is an old saying is computer science, "There are two hard problems in CS, Naming, Cache Invalidation and Off by One Errors."

Don't use for loops, unless you absolutely have to, the tiny perf gain you get isn't worth the debug time (assuming there is any perf gain in this version of .net).

Secondly

new Thread. Don't do that. The creation of a thread is extremely slow and takes up lots of resources, especially for a short lived process like this. Added to the fact, there is overhead in passing data between threads. Use the ThreadPool.QueueUserWorkItem(WaitCallback) instead, if you MUST do short lived threads.

However, as I previously alluded to. Threads are an abstraction for CPU resources. I honestly doubt you are CPU bound. Threading is going to cost you more than you think. Stick to single threads. However you ARE I/O bound, therefore make full usage of asynchronous I/O.

public async Task<IEnumerable<FileModel>> GetExceptionFiles(List<string> foldersPath, List<string> someList)
{
    foreach (var folderPath in foldersPath)        
    foreach (var file in BrowseFiles(folderPath))
    {
         if (false == someList.Any(x => file.Name.Contains(x, StringComparer.InvariantCultureCaseIgnore)))
             continue;
         using (var fileStream = await File.OpenTaskAsync(file.Path, FileMode.Open))
         using (var bufferedStream = new BufferedStream(fileStream))
         using (var streamReader = new StreamReader(bufferedStream))
         ...
             yield return new FileModel();

Collectives™ on Stack Overflow

How can I simplify and optimize this C# code

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related