initialize vector in constructor

[gavin]

morning,

I have a specific "what am I doing wrong" question, also a general
"what can I do better in this code" question.

Here is the code:

class book
{
public:
void setName(char *newBookName);
char *getName();
private:
char *bookName;
};
void book::setName(char *newBookName)
{
bookName = newBookName;
}
char *book::getName()
{
return bookName;
}
class library
{
public:
library(int bookCount)
{
vector<book> books(bookCount);
lastAdd = 0;
}
void addBook(book newBook);
book findBook(char *bookName);
private:
vector<book> books;
int lastAdd;
};

void library::addBook(book newBook)
{
books[lastAdd] = newBook;
lastAdd++;
}
book library::findBook(char *bookName)
{
for(int i = 0; i <= lastAdd; i++)
{
if(bookName == books[i].getName())
{
return books[i];
}
}
return books[0];
}

int main ()
{
library myLibrary = library(5);
book SciFi;
SciFi.setName("Stand On Zanzibar");
myLibrary.addBook(SciFi); // This will throw a memory error
book zanzibar = myLibrary.findBook("Stand On Zanzibar");
return 0;
}

I'm pretty sure that the way I have my vector set up to be dynamically
initalized is not correct. What is the correct way to do this?

Also, is there a better way that I could do this same process? (Have a
generic object that keeps other objects so you can serve them up based
on criteria).

Any help would be greatly appreciated!

[Josh Sebastian]

On Sat, 06 Sep 2003 10:48:53 -0700, gavin wrote:

> morning,
>
> I have a specific "what am I doing wrong" question, also a general
> "what can I do better in this code" question.
>
> Here is the code:
>
> class book
> {
> public:
> void setName(char *newBookName);
> char *getName();
> private:
> char *bookName;
> };

For starters, use std::string instead of C-style strings. After a
quick glance, I bet that'll fix your problem. If you use them
correctly, there's nothing wrong with C-style strings, but you're not
using them correctly (I'll refrain from correcting your usage). They're
very difficult to use -- and usually, there's no reason not to use
std::string instead.

Also, make your code const-correct.

> void book::setName(char *newBookName)
> {
> bookName = newBookName;
> }
> char *book::getName()
> {
> return bookName;
> }
> class library
> {
> public:
> library(int bookCount)
> {
> vector<book> books(bookCount);
> lastAdd = 0;
> }
> void addBook(book newBook);
> book findBook(char *bookName);
> private:
> vector<book> books;
> int lastAdd;
> };
>
> void library::addBook(book newBook)
> {
> books[lastAdd] = newBook;
> lastAdd++;
> }
> book library::findBook(char *bookName)
> {
> for(int i = 0; i <= lastAdd; i++)

ITYM <, not <=.

> {
> if(bookName == books[i].getName())
> {
> return books[i];
> }
> }
> return books[0];
> }

You're using a vector, but you don't actually use it dynamically. Instead
of setting the number of elements in the ctor, just do a push_back in
addBook. This will also make lastAdd unnecessary, as books.size() will
tell you how many books there are.

> int main ()
> {
> library myLibrary = library(5);
> book SciFi;
> SciFi.setName("Stand On Zanzibar");
> myLibrary.addBook(SciFi); // This will throw a memory error book
> zanzibar = myLibrary.findBook("Stand On Zanzibar"); return 0;
> }
> }
> }
> I'm pretty sure that the way I have my vector set up to be dynamically
> initalized is not correct. What is the correct way to do this?
>
> Also, is there a better way that I could do this same process? (Have a
> generic object that keeps other objects so you can serve them up based
> on criteria).

That's what a std::map is for.

Josh

[Blake Kaplan]

gavin wrote:
> morning,
>
> I have a specific "what am I doing wrong" question, also a general
> "what can I do better in this code" question.
>
> Here is the code:

[snip]

> class library
> {
> public:
> library(int bookCount)
> {
> vector<book> books(bookCount);

Here you create a vector of books, coincidentally called "books" that is
created with a size of bookCount, and destroyed at the end of the
constructor. You'll notice that it is *not* the same vector as you
create below.

You have two options:
One is to use an initializer list to call the correct vector constructor
explicitly, this would look like:

library(int bookCount)
: books(bookCount),
lastAdd(0) // initialize both members
{
// nothing to do!
}

Your other option is to call vector::resize in your constructor, however
this is less efficient.

> lastAdd = 0;
> }
> void addBook(book newBook);
> book findBook(char *bookName);
> private:
> vector<book> books;

Here you declare a vector of books that you use whenever you call
"addBook" and "findBook"

> int lastAdd;
> };
>

[snip]

> I'm pretty sure that the way I have my vector set up to be dynamically
> initalized is not correct. What is the correct way to do this?
>

Use the initialization list.

> Also, is there a better way that I could do this same process? (Have a
> generic object that keeps other objects so you can serve them up based
> on criteria).
>

As Josh pointed out, std::map would fill this requirement. His
suggestion of using "push_back" is also a very good one, as this would
allow your list to be expanded infinitely (or until you run out of memory).

--
Blake

[Josh Sebastian]

On Sat, 06 Sep 2003 16:17:10 -0500, Blake Kaplan wrote:

>> library(int bookCount)
>> {
>> vector<book> books(bookCount);
>
> Here you create a vector of books, coincidentally called "books" that is
> created with a size of bookCount, and destroyed at the end of the
> constructor. You'll notice that it is *not* the same vector as you
> create below.

Hah, missed that! Good catch.

Josh

[jeffc]

"gavin" <garbage...@hotmail.com> wrote in message
news:582822a.03090...@posting.google.com...

>
> Also, is there a better way that I could do this same process? (Have a
> generic object that keeps other objects so you can serve them up based
> on criteria).
>
> Any help would be greatly appreciated!

Why are you messing around with char* in C++?