Программа критика моя первая с++ "Привет Мир"


Это моя первая попытка написать простую программу на C++:

#include <iostream>
#include <cstring>
#include <vector>
using namespace std;

class Book {
    public:
    char title[30];

    Book(char *tit)  {
        strcpy(title,tit);
    };

    char *gettitle() {
        return title;
    }
};


int main() {

    Book miolibro((char *)"aaa");
    cout << miolibro.gettitle() << endl;

   std::vector<Book> vettorelibri; 
   for (int i=0; i<10; i++) {
        Book l( (char *)"aaa" );   // i tried something like "aaa"+i but it does not work

        vettorelibri.push_back(l);   
    }

    for(int i=0; i<10; i++) {
        cout << vettorelibri[i].gettitle() << endl;
    }
};


762
10
c++
задан 26 июня 2011 в 04:06 Источник Поделиться
Комментарии
4 ответа

Отмечается, что при работе в C++, вы должны использовать СТД::строка. Если вы хотите остаться с C строками, стилей (или по крайней мере понять, как вы должны работать с ними, в случае, если вам когда-нибудь придется), у меня есть следующие предложения:

strcpy(title,tit);

синица может быть больше, чем буфер название позволяет.[1] в этом случае, ваша программа рухнет, или, что еще хуже, продолжить работу при развращает какой-либо другой части памяти. Чтобы избежать этого, вы всегда должны использовать функции, которые позволяют отслеживать размер буфера назначения и обнаружения переполнения. Например:

strncpy(title, sizeof(title)-1, tit);
title[sizeof(title) - 1] = 0;

Это имеет некоторые нежелательные черты в том, что он обрезает строку, если входной больше, чем пункт назначения. Если вы работаете на системе, которая имеет нестандартные strlcpy, что чище ошибки условия:

if (strlcpy(title, sizeof(title), tit) >= sizeof(title)))
{
// TODO: decide how to handle the error (in your case maybe throw
// an exception)
}

Движемся вдоль спектра (из кода, который падает при больших входных данных, чтобы код, который обрезает входной сигнал, к тому, что код ошибки, если буфер недостаточно...), вы также можете динамически выделить память для этого.

char *title;

Book(const char *tit)
{
title = strdup(tit);
if (!title) { throw std::bad_alloc(); }
}

Это создает осложнения, если забронировать объект не копируется, поэтому вы должны обращаться, что:

Book(const Book &b)
{
title = strdup(b.title);
if (!title) { throw std::bad_alloc(); }
}

И он должен быть освобожден, когда ваш объект будет уничтожен:

~Book()
{
free(title);
}

Если это кажется слишком сложным, это потому, что для высокого уровня C++ кода это, вероятно. СТД::строка будет заботиться о все это для вас. Хотя, обратите внимание, что структура памяти вашего объекта будет существенно отличаться с этими динамической памяти подходы.[2]

Далее...

Book miolibro((char *)"aaa");

Как правило, приходится делать забросы, как это часто показатель того, что вы делаете что-то неправильно. :-) При написании кода и рецензирование работы других людей я склонен попытаться избежать многих ненужных, а возможно скинет. Много раз эти маски ошибок, например литья функцию неправильный указатель на функцию типа может производить сбои, или, отбросив const и может произойти сбой, когда кто-то пытается изменить буфер только для чтения. По причинам, как это хорошая привычка смотреть за ненадлежащее бросает, когда читаешь код, и будьте уверены, чтобы не вводить их самостоятельно; код, как это должно оскорбить ваши чувства. :-) [3]

В вашем случае, проблема в том, что строковые литералы имеют тип константный тип char*, поэтому компилятор жалуется, что вы передаете это то, что не имеет константный. Так что нужно сделать вот это:

Book(const char *tit)
{
// ...
}

Сноски и комментарии по касательной:

[1] Если есть какие-то англоязычные люди, кто может читать ваш код, я предлагаю вам придумать лучшее название, чем синица для этой переменной.

[2] в предыдущем примере, персонажи название существует наряду с другими членами класса. При использовании динамической памяти, есть теперь лишние разыменования указателя происходит до символов. В большинстве случаев это ничего не изменит, но вы можете думать о теоретических случаях, когда прежний желательно за последние.

[3] Примечание: В моем опыте один способ сказать, что кто-то не знает, что они делают в C/C++ является использование отливок. Неаккуратный программист будет писать код, что компилятор отвергает, а потом заставить его "работать" быстро они познакомят бросает в тишине предупреждения компилятора, вместо того чтобы решить актуальную проблему. Если вы работаете в команде, где люди делают это, я считаю, иногда это хороший признак, чтобы быть скептически относятся к работе этого человека по другим причинам.

14
ответ дан 26 июня 2011 в 08:06 Источник Поделиться

Если вы используете C++ вместо C, вы должны использовать СТД::строка вместо типа char массивы. Включают .

8
ответ дан 26 июня 2011 в 05:06 Источник Поделиться

Я знаю, я немного опоздала, но давайте взглянем.

using namespace std;

Нет необходимости для этого. Действительно. Вы уже отборочный большинство ваших имен, продолжайте делать это.

class Book {
public:
char title[30];

Book(char *tit) {
strcpy(title,tit);
};

char *gettitle() {
return title;
}
};

Как уже говорили другие, использовать СТД::строка. Как другие не говорили: наверняка использовать СТД::строка. Вы не можете здраво смешать сырой c строк и исключений, без умных указателей. По сути, смешивая любой динамическое выделение с исключениями без смарт-указатели-это отличный способ вести себя с ума, но в данном случае это даже отдаленно не оправдано, так как есть класс, который делает именно то, что вы хотите.

Теперь, давайте посмотрим, как книга может быть переписана:

class Book {
std::string title;

public:
Book(std::string const& t) : title(t)
{}

std::string get_title() const {
return title;
}
};

Обратите внимание на забавный синтаксис конструктора. Это называется конструктор-инициализатор, и это хорошая идея, чтобы инициализировать объекты, когда это возможно, это может быть быстрее, и это делает его более четким.

Обратите внимание, что строка передается по константной ссылке. Это означает, что вы не копирует его, но вы также не можете изменить это ... вы не хотите его менять, так что ничего страшного. Он имеет еще одно преимущество, но я покажу это позже.

Наконец, мы get_title const значит, что вы можете вызвать его на константный экземпляр книги. Не очень важно в свой минимальный код, но это окупается, чтобы быть как константой исправить , насколько это возможно.

int main() {
Book my_book("aaa");
std::cout << my_book.get_title() << '\n';

Ура, вы знаете о том, как инициализировать объекты, которые не имеют конструктор по умолчанию. Это удобная вещь, чтобы знать, и реже, чем хотелось бы видеть. Обратите внимание, что мы не нуждаемся в бросок больше-на самом деле, мы даже не нужно явно вызывать СТД::строкас конструктором. Это потому, что СТД::строка может быть построен из типа char const и*, которые "ААА" новообращенных. (Если вам интересно, "ААА" не сам чар констит* - это тип char const и[4].)

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

  std::vector<Book> vector_of_books; 
for (int i=0; i<10; i++)
vector_of_books.push_back(Book("aaa"));

Вот код, который делает именно то, что вы делаете, но с книгой's новый конструктор и без временной. Я думаю, что это так ясно, но некоторые могут не согласиться. Еще более четкий способ сделать это

  std::vector<Book> vector_of_books(10, Book("aaa"));

Что буду делать то же самое, и описаны более четко намерениях.

Как в сторону: вы говорите, что "ааа"+я не работал. Это действительно то, что не будет работать, отчасти потому, что c++ не дают все-это-простой способ объединения строк с чем-то еще. Ваше самое лучшее парио должно быть "ААА" + буст::lexical_cast(я), если вы хотели пойти через проблемы явно создать stringstream. Есть также такие функции, как Атой и и snprintf, но те, как правило, менее безопасный или сложнее в использовании.

    for (int i = 0; i < 10; ++i) {
std::cout << vector_of_books[i].get_title() << '\n';
}
};

Этот код прекрасно использовать -- я применил некоторые косметические изменения, но они не особо принципиально. Габриэль предлагаю вам использовать итераторы, но я не думаю, что те делают что-то хорошее:

    for (std::vector<Book>::iterator it = vector_of_books.begin();
it < vector_of_books.end();
++it) {
std::cout << *it << '\n';
}

Код гораздо длиннее и бонус в эффективности не имеет значения. Согласен, с C++11, вы могли бы изменить СТД::вектор::итератор в авто, и тогда это будет достойный, но в C++11, вы могли бы также просто сделать:

    for (std::string const& book : vector_of_books)
std::cout << book << '\n';

Если это кажется запутанным, не волнуйтесь об этом, это самое важное, что вы получите основы вниз (как через СТД::строка), и перебор-это просто хороший вариант, что вы можете иметь в виду, когда это более эффективное/более четким.

8
ответ дан 3 февраля 2012 в 12:02 Источник Поделиться

Есть несколько вопросов:


  • Если вы используете геттер метод титул в книге, вы, вероятно, хотите скрыть поле заголовка, так сделайте его закрытым

  • Если длина названия ограничений в течение 30 является произвольным, возможно, вы захотите изменить название поля на указатель на строку (подробнее о том, какие струны вы действительно хотите здесь: http://en.cppreference.com/w/cpp/string)

  • Если вы хотите использовать книгу конструктор с строковые литералы (например, "Ааа"), вы должны добавить const для параметра тип

  • В книге конструктора и strcpy опасно, потому что он не проверяет длину строки (переполнение буфера возможно), вы должны лучше использовать функції strncpy, или если вы решили использовать константы, вам не придется копировать на все

  • Предпочтительнее использовать верблюжьего или подчеркивания для разделения слов в качестве идентификаторов для удобочитаемости, так переименовать именно gettitle , чтобы именно gettitle или get_title, же относится к miolibro и vettorilibri.

  • Если вы хотите, чтобы избежать литья строковые литералы, вместо того, чтобы использовать константы

  • Если вы хотите передать объект типа, который вы определили в выходной поток (например, cout в вашем случае), желательно перегружать оператор<<.

  • Если вы используете пространства имен (с помощью сайта), Вам не придется вводить все имена, если только имя столкновение возможно, в вашем случае вы можете просто написать вектор , а не СТД::вектор.

  • Правильный способ конструирования нового объекта, чтобы вызвать конструктор, используя оператор new, так что вы должны заменить книгу(...), что неявное преобразование насколько мне известно, с новой книгой(...). Чем должен vettorelibri.push_back(новая книга("ааа" + я)); работает, как ожидалось, поскольку вы переключаетесь на константы (константный тип char* вместо типа char*).

  • Предпочтительный способ итерации по вектору с помощью итератора вместо явного индексации. Вы можете прочитать об этом здесь: https://stackoverflow.com/questions/409348/iteration-over-vector-in-c

Могут быть еще некоторые проблемы, но это должно быть достаточно для начала:).

6
ответ дан 26 июня 2011 в 05:06 Источник Поделиться