Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
Код:
public class HttpStream
{
private readonly HttpHeaders _headers;
private readonly string _body;
internal HttpHeaders Headers
{
get { return _headers; }
}
[CanBeNull]
internal string Body
{
get { return _body; }
}
[CanBeNull]
internal Encoding BodyEncoding { get; private set; }
private HttpStream(StreamReader streamReader, HttpHeaders httpHeaders)
{
if (streamReader == null)
throw new ArgumentNullException("streamReader");
if (httpHeaders == null)
throw new ArgumentNullException("httpHeaders");
_headers = httpHeaders;
_body = readBody(streamReader);
}
internal static HttpStream ParseRequest(StreamReader streamReader)
{
if (streamReader == null)
throw new ArgumentNullException("streamReader");
return new HttpStream(streamReader, new HttpRequestHeaders(streamReader));
}
internal static HttpStream ParseResponse(StreamReader streamReader)
{
if (streamReader == null)
throw new ArgumentNullException("streamReader");
return new HttpStream(streamReader, new HttpResponseHeaders(streamReader));
}
private string readBody(StreamReader streamReader)
{
var contentType = _headers["Content-Type"];
if (!string.IsNullOrWhiteSpace(contentType))
{
foreach (var section in contentType.Split(';'))
{
var parts = section.Split('=');
var name = parts[0].Trim();
switch (name.ToLowerInvariant())
{
case"charset":
BodyEncoding = Encoding.GetEncoding(parts[1].Trim());
break;
}
}
}
var isChunked = false;
var chunked = _headers["Transfer-Encoding"];
if (!string.IsNullOrWhiteSpace(chunked))
{
isChunked = true;
}
Stream compressedStream = null;
Stream deChunkedStream = null;
try
{
var stream = streamReader.BaseStream;
setPositionToBody(stream);
if (isChunked)
{
stream = deChunkeToNewStream(stream);
deChunkedStream = stream;
stream.Position = 0;
}
var contentEncoding = _headers["Content-Encoding"];
if (!string.IsNullOrWhiteSpace(contentEncoding))
{
switch (contentEncoding)
{
case"gzip":
stream = new GZipStream(stream, CompressionMode.Decompress, true);
compressedStream = stream;
break;
case"deflate":
stream = new DeflateStream(stream, CompressionMode.Decompress, true);
compressedStream = stream;
break;
default:
try
{
// путают настройщики серверов Content-Encoding с Content-Typevar dummyEncoding = Encoding.GetEncoding(contentEncoding);
if (BodyEncoding == null)
BodyEncoding = dummyEncoding;
}
catch (ArgumentException)
{
throw new InvalidOperationException("Не известное значение заголовка Content-Encoding:" +
contentEncoding);
}
break;
}
}
using (var bodyReader = new StreamReader(stream, BodyEncoding ?? Encoding.UTF8))
{
return bodyReader.ReadToEnd();
}
}
finally
{
if (deChunkedStream != null)
{
deChunkedStream.Dispose();
}
if (compressedStream != null)
{
compressedStream.Dispose();
}
}
}
private static Stream deChunkeToNewStream(Stream stream)
{
var hexSize = string.Empty;
Stream resStream = new MemoryStream();
int readed;
do
{
var buf = new byte[1];
readed = stream.Read(buf, 0, 1);
if (readed == 1 && buf[0] == 13)
{
readed = stream.Read(buf, 0, 1);
if (readed == 1 && buf[0] == 10)
{
// копируем тело чанка в результирующий потокvar chunkSize = int.Parse(hexSize, NumberStyles.AllowHexSpecifier);
var chunkBuf = new byte[chunkSize];
readed = stream.Read(chunkBuf, 0, chunkSize);
if (readed != chunkSize)
{
throw new InvalidOperationException(
string.Format("Ошибка чтения. Ожидалось байт: {0}; Получено: {1}", chunkSize, readed));
}
resStream.Write(chunkBuf, 0, chunkSize);
hexSize = string.Empty;
// пропускаем CRLF, идущий после каждого чанка
readed = stream.Read(buf, 0, 1);
if (readed == 1 && buf[0] == 13)
{
readed = stream.Read(buf, 0, 1);
if (readed == 1 && buf[0] == 10)
{
continue;
}
}
throw new InvalidOperationException("Чанк не оканчивается на #13#10");
}
throw new InvalidOperationException("Не ожиданный символ #13, без последующего #10");
}
hexSize += (char) buf[0];
} while (readed > 0);
return resStream;
}
private static void setPositionToBody(Stream stream)
{
stream.Position = 0;
var buf = new byte[1];
int readed;
do
{
// читаем до 2 подряд идущих CRLF
readed = stream.Read(buf, 0, 1);
if (readed != 1 || buf[0] != 13)
{
continue;
}
readed = stream.Read(buf, 0, 1);
if (readed != 1 || buf[0] != 10)
{
continue;
}
readed = stream.Read(buf, 0, 1);
if (readed != 1 || buf[0] != 13)
{
continue;
}
readed = stream.Read(buf, 0, 1);
if (readed == 1 && buf[0] == 10)
{
return;
}
} while (readed > 0);
}
}
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
Это просто конфетка по сравнению с тем, с чем приходится работать. Поставил бы твердую 4+.
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
YAO>Код:
В логику сильно не вникал. На первый взгляд — обычный, нормальный код. Оценка 4+
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
Не понимаю, что заставляет людей так думать и писать. Может язык совместно с фрэймворком? Не понимаю, зачем объединять поток и структуру запроса/ответа в один класс. Может только чтобы потом написать application.run().
Оценку ставить не берусь, пока кто-нибудь не объяснит где это так надо.
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
Тут интересный вопрос. Свой худший код, который писался часто в ASAP режиме я оцениваю на 3 или 3- (копипасту я даже в ASAP режиме не допускаю, бывает спускаюсь и ниже удовлетворительного, но в таком состоянии код живет не больше недели, потом меня совесть замучит). Максимальную оценку себе я выше 4 еще не ставил. А на код, которому можно 5 поставить, я бы просто хотел бы посмотреть — всегда есть простор к улучшению .
Уроверь писавшего оцениваю как юниорский. Код бы я оценил на 2, то есть неудовлетворительно, но не смертельно. Тт человека, с опытом меньше двух лет, как показывает практика, лучшего кода ожидать бессмысленно. Это не такая плохая оценка, худший код, который я видел, я б оценил в 0 из 5 (писали американские студенты). А этот можно довольно быстро привести к удовлетворительному состоянию, если поизбавляться от некоторой копипасты, сократить и повысить понятность можно легко, более глубоко даже лень смотреть пока не отрефакторю.
Предполагаю, что это код тестового задания. Соответственно студента, сильному в теории и написавшего подобный код я бы взял на юниора, переучить можно. Если б такое написал кто-то, претендующий на хорошего разработчика — извините, есть куча других контор, которых этот код удовлетворит в полной мере (да там и кодировать не попросят, максимум на бумажке, где по нормальному один хрен не напишешь), у нас он просто не впишется в коллектив.
Если бы мне предложили поддерживать проект с подобным кодом за 200 тысяч в месяц я б обрадовался. За 120 тысяч я б подумал. За 100 тысяч — твердо б не согласился. Но в конторах с таким кодом мне и 50 не предложат .
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
19.08.2011 18:55, elmal пишет:
> Тут интересный вопрос. Свой худший код, который писался часто в ASAP > режиме я оцениваю на 3 или 3- (копипасту я даже в ASAP режиме не > допускаю, бывает спускаюсь и ниже удовлетворительного, но в таком > состоянии код живет не больше недели, потом меня совесть замучит). > Максимальную оценку себе я выше 4 еще не ставил.
Ты бы свой код сюда выложил. Мы бы посмотрели, обос... обсудили.
Posted via RSDN NNTP Server 2.1 beta
Re[2]: А слабо этот говнокод превратить в конфетку?
Здравствуйте, boot, Вы писали:
B>Не понимаю, что заставляет людей так думать и писать. Может язык совместно с фрэймворком? Не понимаю, зачем объединять поток и структуру запроса/ответа в один класс. Может только чтобы потом написать application.run().
Потому что для джуниоров
1 это предельно наглядно, т.к. все еще слабо решают задачи, что бы делать это в уме, соответсвенно код своего рода черновик
2 еще не научились декомпозиции, например выделять определенные обязанности
3 не понимают все имеющиеся проблемы в таком коде
4 не правильно оценивают важность известных им проблем
Здравствуйте, Lloyd, Вы писали:
L>Это просто конфетка по сравнению с тем, с чем приходится работать. Поставил бы твердую 4+.
Вы не шутите? То есть вы считаете, что это вполне нормально:
1. Поведение. Что это за класс вообще? Вроде Stream (HttpStream), но по сути дела никакой не Stream, а скорее Reader. И то сомнительно -- т.к. в нем и заголовки, и тело запроса в виде строки. Поведение размазано.
2. Парсить в этом Stream HTTP-заголовки (хотя есть класс HttpHeaders, HttpRequestHeaders, HttpResponseHeaders):
Вот:
var contentType = _headers["Content-Type"];
if (!string.IsNullOrWhiteSpace(contentType))
{
foreach (var section in contentType.Split(';'))
{
var parts = section.Split('=');
var name = parts[0].Trim();
switch (name.ToLowerInvariant())
{
case"charset":
BodyEncoding = Encoding.GetEncoding(parts[1].Trim());
break;
}
}
}
и вот:
var isChunked = false;
var chunked = _headers["Transfer-Encoding"];
if (!string.IsNullOrWhiteSpace(chunked))
{
isChunked = true;
}
и вот:
var contentEncoding = _headers["Content-Encoding"];
if (!string.IsNullOrWhiteSpace(contentEncoding))
{
switch (contentEncoding)
{
case"gzip":
...
break;
case"deflate":
...
break;
3. Вот такая конструкция:
return new HttpStream(streamReader, new HttpRequestHeaders(streamReader));
как вам? То есть в HttpStream парсим заголовки, а в HttpRequestHeaders и HttpResponseHeaders читаем поток.
4. При чтении в методе readBody закрываем основной поток. При этом иногда он будет закрыт, а иногда нет:
using (var bodyReader = new StreamReader(stream, BodyEncoding ?? Encoding.UTF8))
5. Как вам функция deChunkeToNewStream? Не находите, что это можно написать намного проще?
6. Как вам функция setPositionToBody? И вообще насколько она уместна, если после заголовка всегда идет body?
Я все таки надеюсь что вы не вдавались в подробности. Неужели и вы так пишите (вроде как у вас неплохая подборка званий)?
Здравствуйте, YetAnotherOne, Вы писали:
L>>Это просто конфетка по сравнению с тем, с чем приходится работать. Поставил бы твердую 4+.
YAO>Вы не шутите? То есть вы считаете, что это вполне нормально:
А я смысл не читал, только код посмотрел.
И таки-да, по сравнению с тем, с чем приходится работать, код-таки конфетка.
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
YAO>Код:
YAO>
YAO>
1. Если говорить про качество кода на уровне функции, то это очень хороший код — исключения и работа с ресурсами на месте, всё однообразно в соответствии с некоторыми правилами оформления кода. При желании можно быстро понять и разобраться. 4 из 5.
2. Если смотреть на класс сверху и начинать придиратся, то ни разу не Stream никакой. Да и закрытая readBody(), которая только в конструкторе дергается никак тестированию не поддается. Но на самом деле это — мелочи, которые наоборот только тянет улучшить, но никак не бежать в панике от этого кода, если таковой достанется.
Здравствуйте, YetAnotherOne, Вы писали:
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
Я бы сказал что структура кода конечно не лучшая, видно полное несоблюдение SOLID, функции имееют по 5-8 ответственностей. Но в целом код поддётся простому рефакторингу. По 5-ти бальной системе поставил бы твёрдую 3-ку.
YAO>Как вы считаете, является ли данный класс говнокодом? Если бы вам предложили поддерживать проект с подобным кодом, вы бы были в восторге или огорчились бы? По сравнению с качеством кода в вашей конторе хуже или лучше? Во сколько бы оценили по 5-ти бальной шкале?
Код не очень хорош, есть замечания и по стилю, и по структуре, и по реализации. Но приходилось работать и с намного более плохим кодом, так что все не очень страшно. 4-
Здравствуйте, Vzhyk, Вы писали:
V>Ты бы свой код сюда выложил. Мы бы посмотрели, обос... обсудили.
Код проекта, увы, выкладывать нет желания на всеобщее обозрение. А код проваленного простого тестового задания, на который даже фидбека никакого не получил, как то даже выкладывал здесь
.
Здесь ИМХО ближе к 4-м, старался.
Ну а ASAP реализация одной специфичной задачи, которую оцениваю сам на 3-, и которую возможно потребуется полность переписать или вообще выкинуть (скорее второе):
/**
* Заточенный под поиск свой велосипед. Позволяет искать следующий и предыдущий открытый тег с любого
* места как вперед, так и назад. Собственно даже не XML здесь ищется, а любые теги
*/public class CustomXMLParser {
public TagPosition findPreviousTag(String content, int startPosition) {
return findPreviousTag(content, startPosition, 1);
}
public TagPosition findPreviousTag(String content, int startPosition, int neededToFindOpenTags) {
TagPosition tag = null;
int currentPosition = startPosition;
while (neededToFindOpenTags > 0) {
tag = findPreviousTagPosition(content, currentPosition);
currentPosition = tag.getOpenTagPosition() - 1;
if (tag.isOpenTag()) {
neededToFindOpenTags--;
} else {
neededToFindOpenTags++;
}
}
return tag;
}
public TagPosition findNextTag(String content, int startPosition) {
int neededToFindCloseTags = 1;
TagPosition tag = null;
int currentPosition = startPosition;
while (neededToFindCloseTags > 0) {
tag = findNextTagPosition(content, currentPosition);
currentPosition = tag.getCloseTagPosition() + 1;
if (tag.isOpenTag()) {
neededToFindCloseTags++;
} else {
neededToFindCloseTags--;
}
}
return tag;
}
private TagPosition findPreviousTagPosition(String content, int startPosition) {
TagPosition result = new TagPosition();
result.setOpenTagPosition(0);
result.setCloseTagPosition(startPosition);
boolean found = false;
for (int i = startPosition; i > 0; --i) {
char chr = content.charAt(i);
if (chr == '>') {
result.setCloseTagPosition(i);
if (content.charAt(i - 1) != '/') {
found = true;
}
}
if (chr == '<') {
result.setOpenTagPosition(i);
if (found) {
break;
}
}
}
String foundedTag = content.substring(result.getOpenTagPosition(), result.getCloseTagPosition() + 1);
if (foundedTag.contains("/")) {
result.setOpenTag(false);
} else {
result.setOpenTag(true);
}
return result;
}
private TagPosition findNextTagPosition(String content, int startPosition) {
TagPosition result = new TagPosition();
result.setOpenTagPosition(0);
result.setCloseTagPosition(startPosition);
for (int i = startPosition; i < content.length(); ++i) {
char chr = content.charAt(i);
if (chr == '>') {
result.setCloseTagPosition(i);
if (content.charAt(i - 1) != '/') {
break;
}
}
if (chr == '<') {
result.setOpenTagPosition(i);
}
}
String foundedTag = content.substring(result.getOpenTagPosition(), result.getCloseTagPosition() + 1);
if (foundedTag.contains("/")) {
result.setOpenTag(false);
} else {
result.setOpenTag(true);
}
return result;
}
}
Это пример худшего кода в проекте, хуже этого я не могу припомнить ничего. Косяков я тут сам могу до черта найти, в первую очеред копипаст дикий, а во вторых, это вообще надо все выкинуть нахрен и разбирать XML по человечески.
.
