Лет 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 Takeif (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 Addif (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 TestBegBlockedQueue1using 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 itemsint count = _rnd.Next(50000);
for (int t = 0; t < count; ++t)
_to.Add(_rnd.Next());
_to.CompleteAdding();
// --- read back some itemsint 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 itemsif (_to.Count > 0)
{
while (_to.Count > 0)
_to.Take();
}
Assert.AreEqual(_to.Count, 0);
Console.WriteLine("End test - {0}", MethodBase.GetCurrentMethod().Name);
}
// ----- Test with threadsprivate 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++.
B_>Из того что я вижу тут не try C#, это неиспользование var для локальных переменных, а явное указание типа. Решарпер настойчиво предлагал их использовать, но я не согласился. Что еще тут не так? B_>p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
Префиксы у полей, ну и комментарии отделяющие блоки кода, с C# для этого есть регионы, а так вроде все норм. Имхо, к вам придрались просто по пустяку.
В догонку можете почитать вот это Design Guidelines for Class Library Developers. Правда, не смотря на наличие этого документа, большая часть девелоперов, которых я видел, все равно продолжают считать себя самыми умными и коверкают названия префиксами и постфиксами как хотят. Могу поспорить, что в той пафосной конторке, в которую вы собеседовались имена полей начинаются с какого-нить префикса.
Да, нет со стандартом кодирования там вроде все разумно, в тестовом задании было сказано придерживаться этого http://www.rsdn.ru/article/mag/200401/codestyle.XML
Что я и пытался делать, а что подразумевается под "Префиксы у полей"? они у меня неправильными вроде старался сделать как указано.
B_>p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
Не знаю. На мой взгляд всё вполне читаемо, с учётом того, что ты только перешёл на c# — так вообще зашибись
Здравствуйте, Begemot_, Вы писали:
B_>что подразумевается под "Префиксы у полей"?
подразумевается буквально префиксы у полей, у вас каждое поле начинается со знака подчеркивания, что не рекомендуется делать микрософтом.
Здравствуйте, Begemot_, Вы писали:
B_>Лет 10 пишу в основном на С++ и немного на других языках, C# не пробовал. Предложили работу джуниором на С# честно сказал что его не знаю, но давно думаю освоить. Сказали — мы верим что с вашим опытом вы сможете, напишите тестовое задание, а мы посмотрим. Прочел полкнижки по шарпу, написал задание. Вердикт был в стиле "вы были правы, это написано на с++, а не c#, так что к сожалению не подойдете". B_>Больше комментариев нет, а мне вот интересно что-же так такого страшного? Если бы дело в том что плохо написал задание, там какие-то проблемы или плохой код — я бы понял, но смущает именно упор на том что это "написано не на С#". Кто-нибудь может показать вкратце где тут проблемы и как надо было правильно делать. B_>Задание вкратце "Необходимо реализовать блокирующую очередь и автоматические тесты к ней. Задача должна быть выполнена как можно более простым способом. .Net 3.5 or 4.0, + тесты, + отсутствие проблем с многопоточностью, + правильный стиль етс."
B_>...
B_>Из того что я вижу тут не try C#, это неиспользование var для локальных переменных, а явное указание типа. Решарпер настойчиво предлагал их использовать, но я не согласился. Что еще тут не так?
B_>p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
1) Еще раз убеждаюсь во "вреде" тестовых заданий.
2) Еще раз убеждаюсь в том, что конторы, их дающие надо обходить 10-й дорогой.
ИМХО, код (его стилистика) вполне читаем и хорошо оформлен (логику не смотрел). То, что Вам отказали — скорее плюс, чем минус, потому как см. пункты выше. Когда я шел на вторую работу, я шарпа не знал вообще, а брали именно на шарписта. На собеседовании спрашивали по ООП и не давали никаких тестовых заданий. Думаю, что если у Вас опыт в РЕАЛЬНЫХ 10 лет, то контора сама себе злобная буратино, что отказала за непонятные притянутые за уши причины. С таким опытом и прочитанным Рихтером (по шарпу) Вам можно смело идти мидлом и через минимальное время (когда тонкости языка и фреймворка будут изучены) становится синьйором.
B_>>p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
S>Не знаю. На мой взгляд всё вполне читаемо, с учётом того, что ты только перешёл на c# — так вообще зашибись
Видели бы вы мою первую прогу на С#
Помоему нормально.
Здравствуйте, GGoga, Вы писали:
GG>1) Еще раз убеждаюсь во "вреде" тестовых заданий. GG>2) Еще раз убеждаюсь в том, что конторы, их дающие надо обходить 10-й дорогой.
Вот с этим утверждением я бы поспорил.
Наша контора несколько лет назад отказалась от тестовых заданий, о чём я давно жалею.
Тестовым заданием на раз отсекаются пионеры-теоретики, которые где-то что-то слышали, но как применять не знают. В конце концов, человек-то берётся код писать, а не про ООП рассказывать. Рассказывать-то мнегие мастера.
Я бы тестовое задание давал. И сам я такие делал, когда искад работу.
Конечно, тестовое задание не должно быть на какие-то специальные знания, только если эти знания составляют обязательные требования.
Но вот когда человек, декларирующий хорошее знание SQL не может в уме вычислить результаты простого држойна — это настораживает.
К слову, топикстартера с таким кодом я бы взял, причём даже не на джуниора.
Здравствуйте, vmpire, Вы писали:
V>Вот с этим утверждением я бы поспорил. V>Наша контора несколько лет назад отказалась от тестовых заданий, о чём я давно жалею. V>Тестовым заданием на раз отсекаются пионеры-теоретики, которые где-то что-то слышали, но как применять не знают. В конце концов, человек-то берётся код писать, а не про ООП рассказывать. Рассказывать-то мнегие мастера.
V>Я бы тестовое задание давал. И сам я такие делал, когда искад работу. V>Конечно, тестовое задание не должно быть на какие-то специальные знания, только если эти знания составляют обязательные требования. V>Но вот когда человек, декларирующий хорошее знание SQL не может в уме вычислить результаты простого држойна — это настораживает.
Ок, тогда могу ли я, в ответ на просьбу о написании тестового задания, попросить аванс, чтобы проверить платежеспособность работодателя? Да на такой вопрос работодатель "обезумеет от наглости" соискателя.
А то получается, что в процессе найма только одна тестируемая сторона — соискатель. А то, что работодатель может быть "г...ном" всегда умалчивается.
Куча фирм ищут сотрудников без тестовых заданий и как-то себе выживают и даже деньги зарабатывают. Т.е. получается, что им удается отсеивать плохих кандидатов.
Мне лично все равно, кто и как относится к тестовым заданиям. Я просто даже не пойду на собеседование в фирму, которая просит выполнить такое задание (да, фирмы мечты у меня нет, а на нынешнем рынке труда многие платят одинаковые деньги), потому что под "всего лишь 2 часами потраченного времени" прячется 2 реальных дня, потому что если напишешь за те же 2 часа получишь — "вот здесь можно было бы написать так, сделать еще то-то и то-то, да дописать такое-то", спросив, что этого не было в требованиях — "ну это само по себе подразумевалось..." — занавес...
Почитайте данный форум и найдете кучу тем, где кандидат (ТС не исключение) написал тестовое задание, а его не взяли. Он выставил его здесь "напоказ", а оказывается, что и придраться то не к чему, кроме небольшого отступа и открывающейся фигурной скобки на на отдельной строчке.
Все имхо, но мое отношение к тестам — резко отрицательное. Нервы и личное время дороже и можно вполне устроиться без оного.
Здравствуйте, Visor2004, Вы писали:
V>подразумевается буквально префиксы у полей, у вас каждое поле начинается со знака подчеркивания, что не рекомендуется делать микрософтом.
Да я знаю что MS этого требования нету, но в том стандарте кодирования что мне дали написано что приватные поля класса должны начинаться с "_", и это кстати, единственное что мне там активно не понравилось Ну и решарпер с настройками по умолчанию тоже требует того же. Мне конечно не понравилось, но что делать сказали надо, значит надо. Тем более что отрефакторить готовый код перед отсылкой не сложно
>К слову, топикстартера с таким кодом я бы взял, причём даже не на джуниора.
Ну у меня тоже есть версия что отказали именно по этому, им вроде нужны джуниоры и когда я сказал что шарпа в глаза не видел — сказали, что ничего страшного, подойду. Видимо код их смутил, не стоило так серьёзно относится к тестовому заданию наверное
Здравствуйте, Begemot_, Вы писали:
B_>Да я знаю что MS этого требования нету, но в том стандарте кодирования что мне дали написано что приватные поля класса должны начинаться с "_", и это кстати, единственное что мне там активно не понравилось Ну и решарпер с настройками по умолчанию тоже требует того же. Мне конечно не понравилось, но что делать сказали надо, значит надо. Тем более что отрефакторить готовый код перед отсылкой не сложно
А StyleCop который в дефолтной конфигурации, который некоторые считают тоже рекомендацией от МС — ещё предлагает любой member access (т.е. поля — методы — пофигу), предворять "this." и "base." соответственно.
Префикс "_" — используют очень многие, и это вполне имеет право на жизнь.
Ну а то, что половина BCL имеет в себе члены m_xxxx — то конечно же это ошибки "молодости", когда .NET/BCL был зелёным а гайдов не было — только почему-то в HashSet<T> — всё те же m_buckets/m_count. В BigInteger (System.Numerics) — префикс s_ на статических полях.
Да и чего уже там — префиксы используются в полный рост, не только для полей:
// System.Numerics.BigIntegerinternal int _Sign
{
get { return this._sign; }
}
Это конечно же всё потому, что МС внутри использует тайный coding guidelines, а народу лапшу на уши вешает.
Единственный разуменый критерий — это выработать (или взять уже готовый) один из подходящих стандартов и его придерживаться внутри команды/проекта. Слепо же следовать рекомендациям от МС не нужно, хотя их гайдлайны обязательны для прочтения и достойны рассмотрения в числе первых.
B_>Из того что я вижу тут не try C#, это неиспользование var для локальных переменных, а явное указание типа. Решарпер настойчиво предлагал их использовать, но я не согласился. Что еще тут не так?
B_>p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
1.Проверки на bool. Везде пишете ==false, в C# для булевых переменных не принятно обычно
2.Привычка к for.
3.Привычка к ++i, а не i++.
4.Внутри lock вроде дополнительный Monitor.Wait не нужен.
5.Префиксы режут глаз.
6.Названия переменных не слишком говорящие.
7.Смешение автосвойств и свойств с backing field. Сейчас в принципе наблюдается тенденция использовать автосвойства, если не нужна логика выставления/получения значений дополнительная.
8.Выход из вечного цикла по исключению — как-то не очень кошерно. В целом в C# не принято включать throw как стандартную ветку исполнения кода(вопрос дискуссионный).
9.Непонятен смысл метода CompleteAdding().
10.Для многопоточной записи/выборки объектов используется ConcurrentQueue. При ее использовании куча локов уходит.
Здравствуйте, Begemot_, Вы писали:
B_>Вердикт был в стиле "вы были правы, это написано на с++, а не c#, так что к сожалению не подойдете".
Возможно, они и правы — я бы на вашем месте не особо обижался. Это как хоккеиста учить балету — он всё равно будет пытаться скользить по сцене. (и недоумевать: "ну я же двигаюсь!") Увы, в вас "обижаются" амбиции "сипиписника" — мол, я такой крутой перец, ща вам на любом шарпе сбацаю! А тут опустили как второклассника. Забудьте С++ — это не понты, это ООП-мракобесие на ассемблере.
B_> Кто-нибудь может показать вкратце где тут проблемы и как надо было правильно делать.
Если откровенно, то я даже задания не понял Что такое "блокирующая очередь"? Блокирующая кого, от чего и в какое время? Вы уверены, что правильно поняли задание?
B_>... и автоматические тесты к ней.
Могу успокоить только одним: контора, возлагающая надежды на тесты — тухлое сборище дилетантов. Беда в том, что "тесты" как пилочка для ногтей — где-то незаменима, но в большинстве случаев — бесполезная %#$%#$я.
B_>Сама очередь B_> private readonly Queue<T> _data;
B_> public BegBlockedQueue2() B_> { B_> _data = new Queue<T>(); B_> BoundedCapacity = -1;
Вот это капасити — зачем?? Просто возможностей Queue недостаточно?
B_>p.s. Вопрос не о том сделано ли задание хорошо или плохо, вопрос о стиле кода — что в нем неправильного с точки зрения C# и что показывает на привычку писать на C++.
Да чёрт его знает... у меня код примерно такой же. И подчёркивания для приватных полей тоже юзаю. Просто предположение: вы как-то перемудрили с решением, не поняв задачу. Я даже скроллировать устал, не то, что читать! — возможно, это и послужило каким-то странным намёком на "сипипи головного мозга"
Ещё раз: не обижайтесь, для успешного развития в C# вам придётся начисто забыть C++.
Можно я покоментирую\поспорю, исключительно ради истины?
C>1.Проверки на bool. Везде пишете ==false, в C# для булевых переменных не принятно обычно
В с++ это как раз не тоже принято. Это я ошибся. Когда читал книжку там написано что в отличии от с++, в шарпе нельзя в логических выражениях использовать равенство нулю\не нулю, а надо явно проверять на значение. То есть вместо if (str.Length), надо писать if (str.Length > 0), мне это не понравилось и я запомнил. А когда писал код, почему-то решил что и bool b = true; if (b); тоже нельзя писать, а надо обязательно if (b = true) использовать. Но это каюсь моя ошибка просто, но не как не уши С++, где повторюсь так не пишут.
C>2.Привычка к for.
Чем это плохо? И какие конкретно for в моем коде надо заменить на другой типа цикла?
C>3.Привычка к ++i, а не i++.
это да, чисто из плюсов, ну это автоматически. В принципе при необходимости за неделю можно переучится.
C>4.Внутри lock вроде дополнительный Monitor.Wait не нужен.
ну это зависит от логики приложения В блокирующей очереди нужен иначе работать не будет.
C>5.Префиксы режут глаз.
Префиксы (если вы про _ у приватных полей)определены стандартом кодирования, мне тоже не нравятся, но пришлось сделать.
C>6.Названия переменных не слишком говорящие.
хм, чисто субъективно не согласен, мне все говорящие Но это субъективно, но в любом случае не говорит о том что программа написана "на с++, а не на с#" как сказали мне.
C>7.Смешение автосвойств и свойств с backing field. Сейчас в принципе наблюдается тенденция использовать автосвойства, если не нужна логика выставления/получения значений дополнительная.
Ну везде где было можно я использовал автосвойства, там где было нельзя — писал свои. Или стоило отказаться от автосвойств вообще, если я не могу сделать все свойства автоматическими?
C>8.Выход из вечного цикла по исключению — как-то не очень кошерно. В целом в C# не принято включать throw как стандартную ветку исполнения кода(вопрос дискуссионный).
Пример взят прямо из мсдн. При использовании Take() в блокирующей очереди нет другого свойства выйти из цикла.
C>9.Непонятен смысл метода CompleteAdding().
Видимо вы не писали или не работали с блокирующими очередями Я вот тоже еще 10 дней назад о них не слышал, но сейчас я понимаю зачем нужен CompleteAdding(). И кстати оно слизанно со стандартной реализации блокирующей очереди в .Net 4.0
C>10.Для многопоточной записи/выборки объектов используется ConcurrentQueue. При ее использовании куча локов уходит.
да, у меня первый класс (тут его не показывал) тоже реализовывал задание 5 строчками — был просто пустой класс наследник от стандартной блокирующей очереди, но я решил что такое за тестовое задание не защитают Да и C# попробовать хотелось, поэтому написал свой велосипед — все-таки это тестовое задание...
public class BegBlockedQueue1<T> : BlockingCollection<T>
{
public BegBlockedQueue1() : base(new ConcurrentQueue<T>())
{
}
public BegBlockedQueue1(int boundedCapacity) : base(new ConcurrentQueue<T>(), boundedCapacity)
{
}
}
Здравствуйте, Begemot_, Вы писали:
C>>4.Внутри lock вроде дополнительный Monitor.Wait не нужен. B_>ну это зависит от логики приложения В блокирующей очереди нужен иначе работать не будет.
Объясните плиз, зачем вы используете Pulse-ы и Wait-ы таким образом?
C>>В целом код неплохой довольно. B_>это радует
Согласен что неплохой, и C++-ом не пахнет. Есть вопросы, но они не к коду, а к пониманию lock/Wait и т.п.
Да, а зачем при lock-ах используется ConcurrentQueue? Что за перестраховка?
Здравствуйте, samius, Вы писали:
S>Согласен что неплохой, и C++-ом не пахнет. Есть вопросы, но они не к коду, а к пониманию lock/Wait и т.п. S>Да, а зачем при lock-ах используется ConcurrentQueue? Что за перестраховка?
ТС ConcurrentQueue и не использует.
Здравствуйте, samius, Вы писали:
B_>>ну это зависит от логики приложения В блокирующей очереди нужен иначе работать не будет. S>Объясните плиз, зачем вы используете Pulse-ы и Wait-ы таким образом?
Это блокирующая очередь, она работает следующим образом
1. Первый поток производитель что-то делает, и добавляет задания в очередь используя Add()
2. Второй поток потребитель — берет задания из очереди и обрабатывает их.
При этом
1. если потребитель пытается что-то добавить задачу в очередь которая уже полная (количество задач == максимально разрешенному), то очередь блокирует его выполнение до тех пор, пока потребитель не освободит место в очереди
2. если потребитель, работает быстрее производителя, и пытается взять задачу из пустой очереди, очередь блокирует выполнение производителя до тех пор пока потребитель не добавит задачу.
Отсюда,
public void Add(T item)
{
lock (_locker)
{
while (TryAdd(item) == false)
Monitor.Wait(_locker);
}
}
lock нужен для синхронизации доступа между потоками — обыкновенная реализация потокобезопасности. Дальше мы входим в процедуру TryAdd которая пытается добавить задание не используя блокировку потока, если она вернула False значит очередь переполнена, и мы в Add должны заблокировать поток производителя пока не придет потребитель и не заберет задачу из очереди, освободив нам место. Для этого вызывается Monitor.Wait(_locker); — он снимает lock с объекта давая другому потоку возможность забрать задание и ждет сигнала. Когда получен сигнал — мы опять вызываем TryAdd и так пока не получится добавить.
В свою очередь Take() (вызываемая из потока потребителя) после появления свободного места в очереди вызывает Monitor.Pulse(_locker);, Этот сигнал получает поток потребителя заблокированный Waitом и понимает что нужно еще раз проверить условия while (TryAdd(item) == false) — при успешном добавлении производитель разблокируется.
Точно так же работает Wait в Take() — если нет ничего в очереди, блокируем потребителя, и ждем Pulse от процедуры Add()\TryAdd...