Насколько мой код читабелен?

Question

[AVAtarMod]

Пожалуйста, оцените, насколько мой код читабелен. Потому как учусь в ВУЗе, преподаватели обычно интересуются тем, как бы побольше лаб защитить, одногруппники не дали подробной оценки.
В телеграмм чате программистов спрашивал, понял, что там мало тех, кто разбирается в C++, и складывается ощущение, что сижу в вакууме =).
Поэтому решил спросить здесь, и если у вас найдется время, можете еще оценить, насколько мой код в целом плох/хорош?

Прошу оценить следующие элементы:
1. Сборник библиотек
2. Прикладное использование библиотек.

Если желаете, можете прямо в репозитории оставлять комментарии.

Comments

[Евгений Шатунов]

AVAtarMod , напиши пример, как ты объявишь переменную типа константного указателя на символ.

[AVAtarMod]

Евгений Шатунов,

char a = 'a';
char *const  c_ptr = &a;

[Евгений Шатунов]

AVAtarMod, хорошо. Это очень простой и очень наглядный пример.
Сейчас, за счет склейки квалификатора const и модификатора * ты у читателя украл около секунды времени. Лично мне сейчас, да и практически всем инженерам в таких случаях, приходится вчитываться в странное слово между char и c_ptr.
Среднее время реакции человека - одна секунда. Ты ее крадешь, пряча от читателя модификатор, склеивая его с квалификатором или именем инстанции.

С другой стороны, у тебя в коде присутствуют всякие IntList1D &list, где ты приклеиваешь модификатор к имени. А вот сейчас в коде char *const c_ptr что ты можешь приклеить к имени? А ничего. И выходит что у тебя оформление кода неоднозначное, заставляет все время вчитываться. И чтение твоего кода заставляет тратить на это значительно больше времени чем чтение профессионального кода.

Ты, скорее всего, знаешь что модификаторы имеют отношение к имени инстанции, а не к типу, это верно. Но это не работает там, где каждая миллисекунда твоего внимания стоит много денег. А внимание и время сотрудника очень дорого стоят. Мы используем акценты и экономим время своих сотрудников. В частности, мы делаем акцент именно на оформлении типа: char* const c_ptr.
Все, всего одна правка и акценты уже на местах, имя свободно и легко читается, пишется сразу в подкорку и не тратит время сотрудника.

В своей работе мы руководствуемся C++ Core Guidelines и стандартами оформления кода. Мы стараемся писать самодокументируемый код и ценим внимание сотрудников. Мы следим за тем, чтобы другой человек мог свободно исправить наш код без риска внести в него ошибку по невнимательности.

Хоть ты и отметил решение на свой вопрос, я рекомендую тебе не отказываться от остальных ответов, они тоже являются решениями твоего вопроса. У меня тоже есть что сказать по поводу твоего кода и я оставлю свой ответ когда найду время для него. А пока у тебя есть великолепная возможность пройтись по истории моих ответов и ответов других людей, вдруг ты найдешь что-нибудь полезное для себя.

[AVAtarMod]

Евгений Шатунов, Да, я считаю все ответы решениями, так уж поставлен вопрос. Просто хотел отметить какой-нибудь ответ)

[AVAtarMod]

Евгений Шатунов, Расскажите пожалуйста, какой стиль кода (LLVM, WebKit, Google ...) вы считаете самым эффективным с точки зрения написания кода, который легко читать и поддерживать? (Да, я знаю, что это лишь инструмент из многих). Я выбрал WebKit за основу с некоторыми изменениями, потому как в исходном виде он ужасно непривычен для меня.

[Евгений Шатунов]

AVAtarMod, это было бы полезно оформить в виде отдельного вопроса.
Я думаю, многие люди будут рады высказать свое мнение по этому поводу.

От себя могу сказать довольно коротко. За базу стиля обычно берут Google C++ Code Style [1], [2].
Но у GCCS есть несколько недостатков, т.к. он больше ориентирован на пользователей C пересевших на C++.
Его проблемы выражаются в правилах использования указателей и неиспользования ссылок. В C++ ссылка имеет более важное значение по сравнению с указателем. Принципы работы со ссылками и указателями хорошо объясняются в C++ Core Guidelines.
Исправленный в соответствии с C++ Core Guidelines GCCS будет довольно близким к тому чтобы называться эффективным с точки зрения выразительности кода.

Но есть еще одна важная штука: передача смысла написанного кода. С этим у имеющихся стилей вообще все плохо. Книга Дастина Босуэлла в очень доходчивой форме передает важные приемы для повышения смысловой отдачи кода.

Я использую базу стиля GCCS с указанными выше исправлениями. Однако, свой стиль кода я пока не публикую потому что его оформление - это тоже большая инженерная работа, которую я пока ленюсь сделать.

Answer 1

[Армянское Радио]

Это все конечно очень хорошо, но в репе лежат симлинки вместо файлов. Так что пока оценка -10/10 - достигнут предельный радиус кривизны рук.

Comments

[AVAtarMod]

А как делать так, чтобы изменения файлов библиотеки подхватывались в нужных местах? Или просто include делать до другой папки? Если просто изменить include, то это выглядит менее логично на мой взгляд т.к ломает структуру проектов.

[Армянское Радио]

AVAtarMod,
man git submodule
man git subtree
man cmake

[AVAtarMod]

Армянское Радио, Благодарю)

Answer 2

[Rsa97]

С ходу же:

try
{
    if (from > to)
        throw "Incorrect couple 'from - to' for generating random numbers";
}
catch (const char *message)
{
    std::cerr << message << '\n';
    exit(1);
}

Зачем нужен такой изврат, если вы ловите исключение сразу же? Чем хуже

if (from > to)
{
    std::cerr << message << '\n';
    exit(1);
}

bool init(IntList1D &list, int number)
{
    if (list == nullptr || *list == nullptr)
    {
        if (list == nullptr)
            list = new IntList1D_element *;
        *list = new IntList1D_element(number);
        return true;
    }
    return false;
}

Чем меньше глубина вложенности, тем лучше. Ранний выход из функции лучше, чем выход в самом конце. Сравните

bool init(IntList1D &list, int number)
{
    if (is_initialized(list))
    {
        return false;  
    }
    if (list == nullptr)
    {
        list = new IntList1D_element *;
    }
    *list = new IntList1D_element(number);
    return true;
}

bool is_inited(const IntList1D list)
{
    if (list == nullptr || *list == nullptr)
        return false;
    else
        return true;
}

Здесь и название функции кривоватое и чересчур усложнена сама функция.

bool is_initialized(const IntList1D list)
{
    return (list != nullptr && *list != nullptr)
}

Не соблюдаете единый стайлгайд. В некоторых местах одиночные строки после if взяты в фигурные скобки, в некоторых нет. Выберите один какой-либо стиль и придерживайтесь его. (Hint: добавлять строки гораздо проще, если всегда используются фигурные скобки).

Comments

[Johnny Gat]

Субъективно

if (list == nullptr || *list == nullptr)
    return false;
else
    return true;

более наглядно, чем
return (list != nullptr && *list != nullptr)

[Tony]

Johnny Gat, излишняя наглядность раздувает простой код до безумия.
return list && *list;

[Рональд Макдональд]

Етить у вас терпения. Я вот начал, но дропнул на третьем извращении.

[Johnny Gat]

Tony, с безумием вы явно погорячились.

[Евгений Шатунов]

Johnny Gat, дело в том, что это "субъективно наглядное" изобилует сервисным кодом, который конкретно здесь служит исключительно и только для размытия прямой логики.
Если выражение является предикатом, то зачем для определения булева результата делать булеву проверку результата этого предиката? Это и есть наглядный пример безумия: как легко и просто запутать читателя кода.

[Борис Животное]

Johnny Gat, вообще ни разу не наглядно, на мой взгляд. Я наоборот привык к короткому варианту.

[AVAtarMod]

Благодарю за ответ, классы уже скоро буду изучать и использовать, потому как самому надоело писать в процедурном стиле, когда как классы сильно облегчают логику в данном случае)

Answer 3

[Wataru]

Еще комментарии. Это же С++, а не С?

Вместо набора функций, которые возвращают и принимают IntList1D, стоит завести класс, который содержит в себе указатель на начало списка. У этого класса должны быть методы initalize, is_initialized, pushBack, и т.д. Это сократит код немного и сделает его гораздо более логичным.

Тип элемента списка вы объявляете как приватный класс-член в классе списка, и он не будет засорять пространство имен пользователей вашей библиотеки. Им о нем вообще знать не надо, они хотят в ваш список класть int и получать назад int.

Далее, вы пишите контейнер, в идеале его стоит сделать по образу и подобию стандартных контейнеров. С итераторами и т.д. Чтобы с ними было можно работать точно также. Но, если не потянете, то хоть обзовите методы также, как у стандартных (push_back(), empty(), см std::vector, например).

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

Потом, часть функций называется частично большими буквами, частично строчными. Это вообще зачем? APPEND_FRONT_Link? Соблюдайте единый стиль хотябы в одном названии. Если они отличаются от appendFront тем, что они не предназначены для пользователей библиотек, ну так это как раз аргумент в пользу использования классов. Делаете эту функцию приватным методом. И такие обычно называются с суффиксом Impl (от implementation) - appendFrontImpl.

И вообще чисто большими буквами обычно обзывают макросы, ну или константы на худой конец. Возьмите за основу какой-то стандартный стиль и предерживайтесь его.