Здравствуйте, Nikе, Вы писали:
G>>Как это комментарий дублирует код?
N>Два банальных примера плохого использования комментариев:
N>Комментарий в объявлении:
N>N>// merge 2 blocks into one
N>void merge2blocks(std::vector<line_t> const&block0,std::vector<line_t> const&block1,std::vector<line_t>&result);
N>
N>Дубль + возможно не очень хорошее именование.
А это как раз из области очевидного. Я тут даже только ЗА правильное наименование функции и аргументов. А если совсем уж копнуть то
block_t*result=new(std::nothrow) block_t;
if(result){
result->reserve(block0->size()+block1->size());
merge2blocks(*block0,*block1,*result);
delete block0;
delete block1;
blocks.push(result);
}
Заменить на
std::vector<line_t> merge2blocks(std::vector<line_t> const&block0,std::vector<line_t> const&block1);
block_t*result = merge2blocks(*block0,*block1);
delete block0;
delete block1;
if(result){
blocks.push(result);
}
Все подготовки будут внутри функции, и тогда понятно что мы сливаем два блока в один, который и возвращается из функции.
Здравствуйте, Glas, Вы писали:
G>G>std::vector<line_t> merge2blocks(std::vector<line_t> const&block0,std::vector<line_t> const&block1);
G>
Lines Merge( Lines const& block0, Lines const& block1 );
Хотя и не очевидна допустимость возврата вектора.
Здравствуйте, Nikе, Вы писали:
Я на сеньора по С++ не мечу, так что можно и моим вариантом
Здравствуйте, java_coder771, Вы писали:
A>>Это С-style код с некоторыми фичами С++.
A>>Код на С есть смысл писать только там где нельзя использовать С++ (некоторые микроконтроллеры, драйвера), сортировка строк в файле под виндой — это очевидно не та ситуация.
A>>Как С++ код, это полное гуано. На С++ так не пишут — надо использовать умные указатели, RAII, никакого delete, и уж точно никакого goto cleanup
_>знаешь, интервьюверы нередко (именно в майкрософт и еще нескольких крупных конторах) просят на собеседовании
_>писать код на простом C, чтобы понять суть алгоритма и не отвлекаться на непонятные синтаксические конструкции
непонятные кому?
Здравствуйте, Hydrophobia, Вы писали:
H>От Вас необходимы замечания и конструктивная критика кода.
На вскидку:
1. Приличные люди кладут тестовый пример с данными и .bat-файлом.
2. Дичайшая помесь С-style и С++. Например, вот это — форменный абсурд:
typedef struct{
unsigned int max_size_;// max size in bytes
unsigned int max_threads_;// max threads
std::wstring directory_;// path to temporary directory
} fsort_conf_t;
typedef для структур нужен только для С (и если мне не изменяет ПЗУ, то лишь для достаточно древних компиляторов). Но std::wstring можно вставить только в код на C++, соответственно, typedef тут — явное излишество.
Потом, это, конечно, круто, что ты знаешь, что есть такая штука, как (std::nothrow), но вот ведь беда — std::vector об этом может и не догадываться, т.е. от исключений ты на самом деле
не защищён. Соответственно, скажем, конструкция:
arg->files_.push(out_fname);
запросто может плюнуть исключение, которое не будет перехвачено внутри функции потока (merge2files_thread), то есть ускачет куда-то в системные обработчики.
3. Зачем тут многопоточность? Неужели это способ удовлетворить требование минимального времени? Возможно, что это так, но при
полном отстуствии содержательных комментариев разобраться в алгоритме очень тяжело. При поверхностном взгляде алгоритм смахивает на сортировку слияниями, но она прекрасно реализуется без многопоточности.
4. Какой смысл в макросах FSORT_NAMESPACE_BEGIN/FSORT_NAMESPACE_END ? Ты собираешься поддерживать компиляторы, не поддерживающие namespace? Тогда нельзя будет юзать и
std::wstring.
5. Это что за зверь?
file_?fclose(file_):false;
Трудно написать так:
if (file_){
fclose(file_);
}
?
6. Вот такие вещи из демонстрашек надо убирать:
//*end=0;// remove
7.
#if !defined _FSORT_H_
#define _FSORT_H_ 1
Это что за умничанье на ровном месте?
Ну вот так вот, примерно. В конечном итоге код на самом деле отпугивает безотносительно использованного алгоритма.
Я знаю только две бесконечные вещи — Вселенную и человеческую глупость, и я не совсем уверен насчёт Вселенной. (c) А. Эйнштейн
P.S.: Винодельческие провинции — это есть рулез!
Я знаю только две бесконечные вещи — Вселенную и человеческую глупость, и я не совсем уверен насчёт Вселенной. (c) А. Эйнштейн
P.S.: Винодельческие провинции — это есть рулез!