Здравствуйте, Begemot_, Вы писали:

0)

 //Вот это нафига?
 //Зачем делать блокирующим метод Add()?
 public bool IsAddingCompleted
 public void CompleteAdding()

Вы когда-нибудь использовали очередь оконных сообщений windows?
Сама задача ну очень об этом напоминает.


1) Почему реализованы методы Add() / Take() а не Enqueue() / Dequeue ()?
Это всё-таки очередь!
У вас в стеке тоже будут Add/Take?

2) Метод Take() получился сложным: сложно проследить логику блокировок, из-за того, что TryTake() тоже использует блокировки. Надо проще.
-----------------------------
-----------------------------
В целом код читабельный и вполне понятный.
С таким кодом работать можно, спасибо.
Особенно порадовали пробельные строки между блоками кода внутри методов.

Здравствуйте, Философ, Вы писали:


B_>>>Задача должна быть выполнена как можно более простым способом.
Ф>да, на это стоило обратить внимание, более того, заострить.
дык, я и обратил, в решении было две версии, первая вот

  public class BegBlockedQueue1<T> : BlockingCollection<T>
    {
        public BegBlockedQueue1() : base(new ConcurrentQueue<T>())
        {
        }

        public BegBlockedQueue1(int boundedCapacity) : base(new ConcurrentQueue<T>(), boundedCapacity)
        {
        }
    }

Куда уже более простой способ + полностью реализует что требуется. Хотя согласен, конструкторы тут просто перестраховка от смены используемого хранилища по умолчанию для BlockingCollection<T>, можно без них и это было бы проще



SS>> ... слишком многословным и не удовлетворяющим условию...
Ф>похоже на то
А что слишком многословно? Не считая конструкторов и свойств, в классе всего 5 функций из них 3 решают непосредственную задачу — адд, тейк, CompleteAdding. Две вспомогательные, но их код все равно нужен был поэтому я сделал отдельные функции доступные пользователю, можно было без них — но кода было столько же, то есть я бесплатно получил увеличение функционала + более красивый код. Никогда не думал что это плохо


Ф> //Зачем делать блокирующим метод Add()?
Ну как бы задача у нас реализовать блокирующую очередь...у которой есть вполне четкие задачи что делать и кого когда блокировать.


Ф> //Вот это нафига?
Ф> public bool IsAddingCompleted
Ф> public void CompleteAdding()
опять же — задача реализовать блокирующую очередь. Поставщик должен иметь возможность сообщить что все, работа сделана, больше заданий не будет. Иначе потребитель будет вечно ждать нового задания.


Ф>1) Почему реализованы методы Add() / Take() а не Enqueue() / Dequeue ()?
Ф>Это всё-таки очередь!
Ф>У вас в стеке тоже будут Add/Take?
Ну тут возможно ошибся и стоило писать Enqueue() / Dequeue (), но я слизовал интерфейс и функциональность со стандартной дотнетовской реализации — http://msdn.microsoft.com/en-us/library/dd267312(v=vs.100).aspx зачем выдумывать свое если лучшие собаководы уже все придумали Все методы которые у меня есть Add\Take\TryAdd\TryTake\CompleteAdding и свойства взяты оттуда.
К тому же указанные стандарт кодирования говорит:

"Старайтесь использовать имена с простым написанием. Их легче читать и набирать. Избегайте (в разумных пределах) использования слов с двойными буквами, сложным чередованием согласных. Прежде, чем остановиться в выборе имени, убедитесь, что оно легко пишется и однозначно воспринимается на слух. Если оно с трудом читается, и вы ошибаетесь при его наборе, возможно, стоит выбрать другое." (с) а у меня всегда была проблема с Enqueue/Dequeue, сложно читать\воспринимать на слух




Ф>2) Метод Take() получился сложным: сложно проследить логику блокировок, из-за того, что TryTake() тоже использует блокировки. Надо проще.
Эээ сложно проследить обычную логику потокобезопасности, не входить — пока переменная заблокирована? Ужас, неужели в шарпе все так плохо?
А почему замечания только к Take? Add использует точно такой же алгоритм и такую же "сложную" логику?
И было бы очень интересно посмотреть на более легкую реализацию этой задачи...

И вообще называть называть реализацию такого функционала 177 строчками, из которых больше сотни это пробелы и комментарии сложной... блин я все больше и больше хочу бросить С++ и писать на шарпе



Ф>В целом код читабельный и вполне понятный.
Ф>С таким кодом работать можно, спасибо.
Ф>Особенно порадовали пробельные строки между блоками кода внутри методов.
Ну дык, не для себя писал как обычно, а для людей

Здравствуйте, sysenter, Вы писали:

S>Здравствуйте, Codechanger, Вы писали:

C>>3.Привычка к ++i, а не i++.

S>И что в этом такого неправильного и архивредного, что на этом стоило акцентировать внимание?

Я не говорил, что это неправильно или архивредно

Здравствуйте, Codechanger, Вы писали:

C>1.Проверки на bool. Везде пишете ==false, в C# для булевых переменных не принятно обычно

В С++ тоже не принято для булевых переменных, кстати. Это у ТС наследие pure C.

C>3.Привычка к ++i, а не i++.

Полезная, кстати, привычка. Лучше и в С++ и в С# всегда использовать префиксную форму, чтоб не париться насчет порядка выполнения при передаче в методы.
Вот например, C#:

        delegate int del(int n);

        static void f(del d)
        {
            int x = d(1);
            Console.WriteLine("Res = " + x.ToString());
        }

        static void Main(string[] args)
        {
            f( n => n++ );

            Console.ReadKey();
        }

На выходе будет 1, а не ожидаемое 2. А тот, кто привык к плюсАм, напишет ++n и получит на выходе 2.

B_>        /// <summary>
B_>        /// Removes an item from the BegBlockedQueue2.
B_>        /// </summary>
B_>        /// <returns>The item removed from the collection..</returns>
B_>        public T Take()
B_>        {
B_>            lock (_locker) // <<<<<<<<<<<<<<<<<
B_>            {
B_>                T item;
B_>                while (TryTake(out item) == false)
B_>                {
B_>                    if (IsCompleted)
B_>                        throw new InvalidOperationException(); // Эээээээ????? Вызываем Take и на ровном месте получаем InvalidOperationException????????

B_>                    Monitor.Wait(_locker); // <<<<<<<<<<<<<<<<<<<<<??????????
B_>                }

B_>                return item;
B_>            }
B_>        }
B_>    }
B_>}

WTF???????

PS:
Это просто жЭсточайший код.

Здравствуйте, Tom, Вы писали:

Tom>WTF???????

Tom>PS:
Tom>Это просто жЭсточайший код.

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

Здравствуйте, Tom, Вы писали:

// Эээээээ????? Вызываем Take и на ровном месте получаем InvalidOperationException????????

Tom>WTF???????

    class Program
    {
        static void Main(string[] args)
        {
            BegBlockedQueue2<int> queue = new BegBlockedQueue2<int>();
            queue.Add(1);
            //queue.CompleteAdding();
            while (!queue.IsCompleted)
            {
                Console.WriteLine(queue.Take());
            }
            Console.WriteLine("т.к. добавление не завершено, мы эту строку никогда не увидим");
            Console.ReadLine();
        }
    }

Что я делаю не так?

Здравствуйте, Философ, Вы писали:


Ф>Что я делаю не так?
может быть не правильно используете блокирующую очередь?

Здравствуйте, Tom, Вы писали:

Tom>PS:
Tom>Это просто жЭсточайший код.

Это не код, это жизнь у blocking collection такая жестокая. Вот аналог из BCL:

// System.Collections.Concurrent.BlockingCollection<T>
public T Take()
{
    T result;
    if (!this.TryTake(out result, -1, CancellationToken.None))
    {
        throw new InvalidOperationException(SR.GetString("BlockingCollection_CantTakeWhenDone"));
    }
    return result;
}

Здравствуйте, Begemot_

Простейшая блокирующая очередь:

public sealed class Queue<T>
{
    private readonly System.Collections.Generic.Queue<T> queue = new System.Collections.Generic.Queue<T>();

    public void Enqueue (T x)
    {
        lock (this)
        {
            this.queue.Enqueue(x);
            System.Threading.Monitor.PulseAll(this);
        }
    }

    public T Dequeue ()
    {
        lock (this)
        {
            while (this.queue.Count == 0)
            {
                System.Threading.Monitor.Wait(this);
            }
            return this.queue.Dequeue();
        }
    }
}

Здравствуйте, Сергей Губанов, Вы писали:

СГ>Простейшая блокирующая очередь:

Да, простейшая, у меня более функциональная — поддерживает максимальный размер очереди и соответственно блокировку не только потребителя, но и поставщика.

Здравствуйте, Begemot_, Вы писали:

B_>Да, простейшая, у меня более функциональная — поддерживает максимальный размер очереди и соответственно блокировку не только потребителя, но и поставщика.

Простейшая блокирующая очередь с фиксированной ёмкостью блокирующая не только потребителя, но и поставщика:

public sealed class Queue<T>
{
    private readonly System.Collections.Generic.Queue<T> queue = new System.Collections.Generic.Queue<T>();
    private readonly int capacity;

    public Queue (int capacity)
    {
        if (capacity <= 0)
        {
            throw new System.ArgumentException("Invalid capacity");
        }
        this.capacity = capacity;
    }

    public void Enqueue (T x)
    {
        lock (this)
        {
            while (this.queue.Count == this.capacity)
            {
                System.Threading.Monitor.Wait(this);
            }
            this.queue.Enqueue(x);
            System.Threading.Monitor.PulseAll(this);
        }
    }

    public T Dequeue ()
    {
        lock (this)
        {
            while (this.queue.Count == 0)
            {
                System.Threading.Monitor.Wait(this);
            }
            System.Threading.Monitor.PulseAll(this);
            return this.queue.Dequeue();
        }
    }
}

Здравствуйте, Сергей Губанов, Вы писали:


СГ>Простейшая блокирующая очередь с фиксированной ёмкостью блокирующая не только потребителя, но и поставщика:
Ну и к чему это код? у меня точно такое же решение. Правда немного по другому выглядит, за счет дополнительной функциональности — но суть та же, что хотели сказать то?

К тому же у вас нет возможности сообщить что производитель закончил создавать задания надо закруглятся, потребитель запросит очередное задание и заблокируется навечено...

Здравствуйте, Rudolf, Вы писали:

R>Здравствуйте, Codechanger, Вы писали:

C>>1.Проверки на bool. Везде пишете ==false, в C# для булевых переменных не принятно обычно

R>В С++ тоже не принято для булевых переменных, кстати. Это у ТС наследие pure C.

C>>3.Привычка к ++i, а не i++.

R>Полезная, кстати, привычка. Лучше и в С++ и в С# всегда использовать префиксную форму, чтоб не париться насчет порядка выполнения при передаче в методы.
R>Вот например, C#:
R>

R>        delegate int del(int n);

R>        static void f(del d)
R>        {
R>            int x = d(1);
R>            Console.WriteLine("Res = " + x.ToString());
R>        }

R>        static void Main(string[] args)
R>        {
R>            f( n => n++ );

R>            Console.ReadKey();
R>        }
R>

R>На выходе будет 1, а не ожидаемое 2. А тот, кто привык к плюсАм, напишет ++n и получит на выходе 2.

Вот drol несогласен с моим постингом. А с чем конкретно?

Monitor.Wait внутри лока и PulseAll внутри лока, вы уверены, что это будет работать?

Здравствуйте, Аноним, Вы писали:

А>Monitor.Wait внутри лока и PulseAll внутри лока, вы уверены, что это будет работать?

Абсолютно уверен. Это монитор Хоара.

Monitor.Wait и Monitor.PulseAll разрешается вызывать только внутри лока. В этом весь смысл.

Пока поток спит внутри Monitor.Wait он не лочит монитор, Monitor.Wait — это временный выход из лока.

Здравствуйте, Begemot_, Вы писали:

B_>К тому же у вас нет возможности сообщить что производитель закончил создавать задания надо закруглятся, потребитель запросит очередное задание и заблокируется навечено...

Это мелочовочка. У тебя код очень громоздкий, тяжко читать, а у Губанова в 5 сек.

Здравствуйте, Ikemefula, Вы писали:

I>Это мелочовочка. У тебя код очень громоздкий, тяжко читать, а у Губанова в 5 сек.
Ну да, любая сложная задача имеет простое, легкое для понимания неправильное решение

Здравствуйте, Sinix, Вы писали:

I>>Это мелочовочка. У тебя код очень громоздкий, тяжко читать, а у Губанова в 5 сек.
S>Ну да, любая сложная задача имеет простое, легкое для понимания неправильное решение

Где ошибка у Губанова ?

Здравствуйте, Ikemefula, Вы писали:

I>Где ошибка у Губанова?
Неправильное решение необязательно означает ошибку С кодом всё в порядке, просто он, в отличие от кода топикстартера, не покрывает типовых сценариев для BlockingCollection. Конечно, можно похоливарить на тему "коллекция, блокирует — значит всё ок", но это будет чистейшей воды сфероконизм.

Достаточно добавить минимум необходимого — TryAdd/TryTake + возможность закрытия очереди, и получим примерно одинаковый по простоте и читаемости код.