Уши C++ или C++ style vs C# style
От: Begemot_ Россия http://softvoile.com/
Дата: 29.08.12 12:02
Оценка: 14 (2)
Лет 10 пишу в основном на С++ и немного на других языках, C# не пробовал. Предложили работу джуниором на С# честно сказал что его не знаю, но давно думаю освоить. Сказали — мы верим что с вашим опытом вы сможете, напишите тестовое задание, а мы посмотрим. Прочел полкнижки по шарпу, написал задание. Вердикт был в стиле "вы были правы, это написано на с++, а не c#, так что к сожалению не подойдете".

Больше комментариев нет, а мне вот интересно что-же так такого страшного? Если бы дело в том что плохо написал задание, там какие-то проблемы или плохой код — я бы понял, но смущает именно упор на том что это "написано не на С#". Кто-нибудь может показать вкратце где тут проблемы и как надо было правильно делать.

Задание вкратце "Необходимо реализовать блокирующую очередь и автоматические тесты к ней. Задача должна быть выполнена как можно более простым способом. .Net 3.5 or 4.0, + тесты, + отсутствие проблем с многопоточностью, + правильный стиль етс."


Главный класс

using System;
/* 
 * ReadMe
 * 
 * Так как задача должна быть выполнена как можно более простым способом, был реализован класс BegBlockedQueue1<T> 
 * который решает задачу наиболее простым способом.
 * 
 * Класс BegBlockedQueue2<T> это собственная реализация блокирующей очереди, частично совместимой со стандартной ConcurrentQueue<T>
 * опять же, помня про требование простейшего способа, поддерживается только самый просто режим работы - 1 производитель \1 потребитель 
 * BegBlockedQueue2<T> не реализует интерфейс IProducerConsumerCollection<T> для возможного использования с .Net 3.5 (и возможно более ранними версиями, не тестировалось)
 * 
 * Оба класса успешно проходят весь набор тестов.
 * Переключить тестирование между классами BegBlockedQueue1<T> и BegBlockedQueue2<T> можно комментируя\раскоментирую строчку 
 * #define TestBegBlockedQueue1
 * в файле UnitTest.cs
 */

namespace BegBlockedQueue
{
    internal class Program
    {
        private static void Main()
        {
            Test1();
            Test2();
        }


        private static void Test1()
        {
            Console.WriteLine(" --- Start Test1");
            BegBlockedQueue1<int> q = new BegBlockedQueue1<int>();

            q.Add(2);
            q.Add(32);
            q.Add(212);
            q.CompleteAdding();

            Console.WriteLine("count items in collection = {0}", q.Count);

            try
            {
                while (true) Console.WriteLine(q.Take());
            }
            catch (InvalidOperationException)
            {
                Console.WriteLine("That's All!");
            }

            Console.WriteLine(" --- End Test1");
        }


        private static void Test2()
        {
            Console.WriteLine(" --- Start Test2");
            BegBlockedQueue2<int> q = new BegBlockedQueue2<int>(5);

            q.Add(66);
            q.Add(67);
            q.Add(68);
            q.CompleteAdding();
            Console.WriteLine("count {0} ({1})", q.Count, q.BoundedCapacity);

            try
            {
                //int t; while (q.TryTake(out t)) Console.WriteLine(t);
                while (true) Console.WriteLine(q.Take());
            }
            catch (InvalidOperationException)
            {
                Console.WriteLine("That's All!");
            }

            Console.WriteLine(" --- End Test2");
        }
    }
}


Сама очередь

using System;
using System.Collections.Generic;
using System.Threading;

namespace BegBlockedQueue
{
    ///<remarks> 
    ///  Own realization of blocked queue class, testing in .Net 3.5 and 4.0
    ///  Note: support only single producer/single consumer mode of work
    ///</remarks>
    public class BegBlockedQueue2<T>
    {
        private readonly Queue<T> _data;
        private readonly object _locker = new object();
        private bool _isAddingCompleted = false;
        

        // ------------- Constructors

        /// <summary>
        /// Initializes a new instance of the BegBlockedQueue2 class without an upper-bound.
        /// </summary>
        public BegBlockedQueue2()
        {
            _data = new Queue<T>();
            BoundedCapacity = -1;
        }

        /// <summary>
        /// Initializes a new instance of the BegBlockedQueue2 class with the specified upper-bound.
        /// </summary>
        /// <param name="boundedCapacity"></param>
        public BegBlockedQueue2(int boundedCapacity)
        {
            _data = new Queue<T>(boundedCapacity);
            BoundedCapacity = boundedCapacity;
        }


        // ------------- Properties

        /// <summary>
        /// Gets the number of elements contained in the BegBlockedQueue2.
        /// </summary>
        public int Count
        {
            get { return _data.Count; }
        }

        /// <summary>
        /// Gets whether this BegBlockedQueue2 has been marked as complete for adding.
        /// </summary>
        public bool IsAddingCompleted
        {
            get { return _isAddingCompleted; }
            protected set { _isAddingCompleted = value; }
        }

        /// <summary>
        /// Gets whether this BegBlockedQueue2 has been marked as complete for adding and is empty.
        /// </summary>
        public bool IsCompleted
        {
            get { return _isAddingCompleted && Count == 0; }
        }

        /// <summary>
        /// Gets the bounded capacity of this BegBlockedQueue2 instance
        /// </summary>
        public int BoundedCapacity { get; protected set; }


        // ------------- Methods

        /// <summary>
        /// Marks the BegBlockedQueue2 instances as not accepting any more additions.
        /// </summary>
        public void CompleteAdding()
        {
            lock (_locker)
            {
                IsAddingCompleted = true;

                // we should notice consumer thead if it waiting for adding new item inside Take()
                Monitor.Pulse(_locker);
            }
        }

        /// <summary>
        /// Try to add the item to to BegBlockedQueue2, 
        /// </summary>
        /// <param name="item">The item to be added to the collection.</param>
        /// <exception cref="InvalidOperationException"></exception>
        /// <returns>True if item was added to collection, otherwise false.</returns>
        public bool TryAdd(T item)
        {
            if (IsAddingCompleted)
                throw new InvalidOperationException("The collection has been marked as complete with regards to additions.");

            lock (_locker)
            {
                if (BoundedCapacity != -1 && Count >= BoundedCapacity)
                    return false;

                _data.Enqueue(item);

                // Maybe there is waiting consumer inside Take
                if (Count == 1)
                    Monitor.Pulse(_locker);

                return true;
            }
        }


        /// <summary>
        /// Add the item to to BegBlockedQueue2, may block producer until space is available to store the provided item.
        /// </summary>
        /// <param name="item">The item to be added to the collection.</param>
        /// <exception cref="InvalidOperationException"></exception>
        public void Add(T item)
        {
            lock (_locker)
            {
                while (TryAdd(item) == false)
                    Monitor.Wait(_locker);
            }
        }

        /// <summary>
        /// Tries to remove an item from the BegBlockedQueue2.
        /// </summary>
        /// <param name="item">The item to be removed from the collection.</param>
        /// <returns>true if an item could be removed; otherwise, false.</returns>
        public bool TryTake(out T item)
        {
            lock (_locker)
            {
                if (Count == 0)
                {
                    item = default(T);
                    return false;
                }

                item = _data.Dequeue();

                // Maybe there is waiting producer inside Add
                if (IsAddingCompleted == false && Count == BoundedCapacity - 1)
                    Monitor.Pulse(_locker);

                return true;
            }
        }


        /// <summary>
        /// Removes an item from the BegBlockedQueue2.
        /// </summary>
        /// <returns>The item removed from the collection..</returns>
        public T Take()
        {
            lock (_locker)
            {
                T item;
                while (TryTake(out item) == false)
                {
                    if (IsCompleted)
                        throw new InvalidOperationException();

                    Monitor.Wait(_locker);
                }

                return item;
            }
        }
    }
}


Тесты


// Use this for switching between BegBlockedQueue1 and BegBlockedQueue2
// if this line is commented  BegBlockedQueue2 will be used for testing instead of BegBlockedQueue1
//#define TestBegBlockedQueue1

using System;
using System.Reflection;
using System.Threading;
using NUnit.Framework;
using BegBlockedQueue;

namespace UnitTest
{
#if TestBegBlockedQueue1
    using TestClassImplementation = BegBlockedQueue1<int>;
#else
    using TestClassImplementation = BegBlockedQueue2<int>;
#endif

    [TestFixture]
    public class TestClass
    {
        private TestClassImplementation _to;
        private Random _rnd;
        private int _maxCountDataInThreadTest = 1000;
        private int _countAddedDataInThreadTest;
        private int _countTakenDataInThreadTest;

        // ---------

        [TestFixtureSetUp]
        public void TestSetup()
        {
            _to = new TestClassImplementation();
            _rnd = new Random((int)DateTime.Now.Ticks & 0x0000FFFF);
        }

        [TearDown]
        public void FinalizeTest()
        {
            _to = new TestClassImplementation();
        }


        // ---------


        [Test]
        public void TryAddFewItems()
        {
            Console.WriteLine("Start test - {0}", MethodBase.GetCurrentMethod().Name);

            // --- recreate queue with upper-bound limit
            _to = new TestClassImplementation(10);

            // --- TryAdd a few items
            Assert.AreEqual(_to.Count, 0);
            Assert.AreEqual(_to.BoundedCapacity, 10);

            _to.TryAdd(_rnd.Next());
            Assert.AreEqual(_to.Count, 1);

            for (int t = 0; t < 20; ++t)
                _to.TryAdd(_rnd.Next());

            Assert.AreEqual(_to.Count, _to.BoundedCapacity);

            Console.WriteLine("Really added {0} items", _to.BoundedCapacity);
            Console.WriteLine("End test - {0}", MethodBase.GetCurrentMethod().Name);
        }


        [Test]
        public void TryTakeFewItems()
        {
            int item;
            Assert.AreEqual(_to.Count, 0);
            Assert.AreEqual(_to.TryTake(out item), false);

            int[] array = new int[20];

            for (int t = 0; t < 20; ++t)
            {
                array[t] = _rnd.Next();
                _to.TryAdd(array[t]);
                Console.WriteLine("Added {0}", array[t]);
            }

            for (int t = 0; t < 20; ++t)
            {
                Assert.AreEqual(_to.TryTake(out item), true);
                Assert.AreEqual(item, array[t]);
                Console.WriteLine("    Taken {0}", item);
            }

            Assert.AreEqual(_to.TryTake(out item), false);
            Assert.AreEqual(item, default(int));
        }

        [Test]
        public void TestIsCompleted()
        {
            Assert.AreEqual(_to.IsAddingCompleted, false);
            Assert.AreEqual(_to.IsCompleted, false);

            Assert.AreEqual(_to.TryAdd(0), true);
            _to.CompleteAdding();
            Assert.AreEqual(_to.IsAddingCompleted, true);
            Assert.AreEqual(_to.IsCompleted, false);

            try
            {
                _to.TryAdd(0);
                Assert.Fail();
            }
            catch (InvalidOperationException)
            {
            }

            int item;
            Assert.AreEqual(_to.TryTake(out item), true);
            Assert.AreEqual(_to.IsAddingCompleted, true);
            Assert.AreEqual(_to.IsCompleted, true);
        }


        [Test]
        public void AddFewItems()
        {
            Console.WriteLine("Start test - {0}", MethodBase.GetCurrentMethod().Name);

            // --- Add a few items
            Assert.AreEqual(_to.Count, 0);
            _to.Add(_rnd.Next());
            Assert.AreEqual(_to.Count, 1);

            int count = _rnd.Next(50000);
            for (int t = 0; t < count; ++t)
                _to.Add(_rnd.Next());
            _to.CompleteAdding();

            Console.WriteLine("Added {0} items", count + 1);

            Assert.AreEqual(_to.Count, count + 1);

            Console.WriteLine("End test - {0}", MethodBase.GetCurrentMethod().Name);
        }


        [Test]
        public void AddAndGetFewItems()
        {
            Console.WriteLine("Start test - {0}", MethodBase.GetCurrentMethod().Name);

            // --- Add a few items
            int count = _rnd.Next(50000);
            for (int t = 0; t < count; ++t)
                _to.Add(_rnd.Next());
            _to.CompleteAdding();

            // --- read back some items
            int countTaked = _rnd.Next(count);
            for (int t = 0; t < countTaked; ++t)
                _to.Take();

            Console.WriteLine("Added {0} items, taked {1}, still in queue {2} (really {3})", count, countTaked,
                              count - countTaked, _to.Count);
            Assert.AreEqual(_to.Count, count - countTaked);

            // --- read all items
            if (_to.Count > 0)
            {
                while (_to.Count > 0)
                    _to.Take();
            }
            Assert.AreEqual(_to.Count, 0);

            Console.WriteLine("End test - {0}", MethodBase.GetCurrentMethod().Name);
        }

        // ----- Test with threads
        private void AddInThread(object data)
        {
            _countAddedDataInThreadTest = _rnd.Next(_maxCountDataInThreadTest/2, _maxCountDataInThreadTest);
            for (int t = 0; t < _countAddedDataInThreadTest; ++t)
            {
                int i = _rnd.Next(1000);
                _to.Add(i);
                Console.WriteLine("Added {0}", i);
                if ((int)data > 0) Thread.Sleep((int)data);
            }

            _to.CompleteAdding();
        }

        private void TakeInThread(object data)
        {
            try
            {
                while (true)
                {
                    if ((int)data > 0) Thread.Sleep((int)data);
                    int i = _to.Take();
                    Console.WriteLine("    Taken {0}", i);
                    ++_countTakenDataInThreadTest;
                }
            }
            catch (InvalidOperationException)
            {
                Console.WriteLine("Taked all data");
            }
        }

        [Test]
        public void TestInThreads()
        {
            // --- recreate queue with upper-bound limit
            _to = new TestClassImplementation(10);

            _countAddedDataInThreadTest = _countTakenDataInThreadTest = 0;
            Thread tAdd = new Thread(AddInThread);
            tAdd.Start(0);
            Thread tTake = new Thread(TakeInThread);
            tTake.Start(0);

            tAdd.Join();
            tTake.Join();

            Console.WriteLine("TestInThread: added {0}, taken {1} items", _countAddedDataInThreadTest, _countTakenDataInThreadTest);
            Assert.AreEqual(_countAddedDataInThreadTest, _countTakenDataInThreadTest);
        }

        [Test]
        public void TestInThreadsForceProducerBlock()
        {
            // --- recreate queue with upper-bound limit
            _to = new TestClassImplementation(5);
            _maxCountDataInThreadTest = 50;

            _countAddedDataInThreadTest = _countTakenDataInThreadTest = 0;
            Thread tAdd = new Thread(AddInThread);
            tAdd.Start(0);
            Thread tTake = new Thread(TakeInThread);
            tTake.Start(200);

            tAdd.Join();
            tTake.Join();

            Console.WriteLine("TestInThread: added {0}, taken {1} items", _countAddedDataInThreadTest, _countTakenDataInThreadTest);
            Assert.AreEqual(_countAddedDataInThreadTest, _countTakenDataInThreadTest);
        }

        [Test]
        public void TestInThreadsForceConsumerBlock()
        {
            // --- recreate queue with upper-bound limit
            _to = new TestClassImplementation(5);
            _maxCountDataInThreadTest = 50;

            _countAddedDataInThreadTest = _countTakenDataInThreadTest = 0;
            Thread tTake = new Thread(TakeInThread);
            tTake.Start(0);
            Thread tAdd = new Thread(AddInThread);
            tAdd.Start(200);

            tAdd.Join();
            tTake.Join();

            Console.WriteLine("TestInThread: added {0}, taken {1} items", _countAddedDataInThreadTest, _countTakenDataInThreadTest);
            Assert.AreEqual(_countAddedDataInThreadTest, _countTakenDataInThreadTest);
        }
    }
}


Из того что я вижу тут не try C#, это неиспользование var для локальных переменных, а явное указание типа. Решарпер настойчиво предлагал их использовать, но я не согласился. Что еще тут не так?

p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
--
Блог шароварщика ::Микроблог про wxWidgets
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.