Странное место для обработки потоков?

#c# #asp.net #multithreading

#c# #asp.net #многопоточность

Вопрос:

Я поддерживаю некоторые существующие страницы и наткнулся на фрагмент кода из System.Threading , и я не уверен, что с этим делать.

(в этом суть)

 protected void Page_Load(object sender, EventArgs e)
{
    XmlDocument xmlDoc = GetFromCMS();
    var nodes = xmlDoc.SelectNodes("/xpath/to/nodes");

    int i = 0;
    while (i < nodes.Count)
    {
        //do stuff with nodes[i]

        //last line in while loop - this is where I'm confused
        Math.Max(System.Threading.Interlocked.Increment(ref i), i - 1);
    }
}

Имеет ли смысл делать что-то подобное? Нельзя ли вместо этого i увеличить ala i ? Я не разбираюсь в многопоточности, но, учитывая, что на этой странице нет другого многопоточного кода и ничего действительно «особенного» не происходит (не создаются дополнительные потоки и т.д.), Мне это кажется немного странным.

Спасибо за вашу помощь!

1. Math.Max тоже бесполезен, все это можно переписать в ‘for (int i = 0; i < узлы. Считайте; i ){…}’

2. @SelflessCoder вы правы, этот рефакторинг тоже пропустил.

3. Похоже на ошибку вырезания и вставки

4. похоже, кто-то не знает, что делает!

5. @mitch на нашем сайте так много подобного мусора от других, менее осторожных разработчиков. Но в этом-то и прикол, верно? Как исправить эти маленькие жемчужины?

Ответ №1:

Ваша интуиция верна — код немного странный, вероятно, его можно было бы неплохо представить в DailyWTF.

Я не уверен в первоначальных мотивах разработчиков, но без какого-либо другого контекста метод кажется потокобезопасным. Вы должны иметь возможность увеличивать i использование i ; без какого-либо риска.

Еще лучше, вы можете устранить это, i переписав как foreach вместо:

 protected void Page_Load(object sender, EventArgs e)
{
    XmlDocument xmlDoc = GetFromCMS();
    var nodes = xmlDoc.SelectNodes("/xpath/to/nodes");

    foreach(var node in nodes)
    {
        //do stuff with node
    }
}

Ответ №2:

i является локальной переменной (и не используется совместно ни с какими другими потоками), поэтому простой i безопасен.

Итак, замените это:

 Math.Max(System.Threading.Interlocked.Increment(ref i), i - 1);

с этим

 i  ;

Или, как указал комментатор, замените простым циклом for или foreach !

Ответ №3:

Если вы каким-то образом не делитесь локальной i переменной с другими потоками (что весьма сомнительно), i будет работать так же хорошо, без накладных расходов на функцию приращения блокировки.

Единственный способ, которым я мог бы увидеть, что это происходит, — это если вы передаете i по ссылке внутри тела цикла в другую функцию, которая затем поделилась бы этим. Если он используется только локально, все в порядке.