Sign in to follow this  
Plasmarobo

Last resort!

Recommended Posts

Plasmarobo    100
This is my last resort before I just give it up. What does this mean and how do I fix it? Should I use pointers?
//Item structure .h
#include <fstream>
#include <string>
#include <queue>
//This is the structure of an Item


using namespace std;

class ItemsData
{
      protected:

struct item
 {
      int id;
      string name;
      int wieght;
      int value;
      char type;
      int para1;
      int val1;
      int para2;
      int val2;
      bool key;
};

private:
queue<item> ItemDataBase;

public:
       
ItemsData(){LoadItems();}


void MakeItem(string iname, int iwieght, int ival, char type)
{
    item nItem;
    item hItem = ItemDataBase.back();
    nItem.id = hItem.id + 1;
    nItem.name = iname;
    nItem.wieght = iwieght;
    nItem.value = ival;
    nItem.type = type;
    nItem.key = false;
    ItemDataBase.push(nItem);
}

void MakeItem(string iname, int iwieght, int ival, char type, bool key)
{
    item nItem;
    item hItem = ItemDataBase.back();
    nItem.id = hItem.id + 1;
    nItem.name = iname;
    nItem.wieght = iwieght;
    nItem.value = ival;
    nItem.type = type;
    nItem.key = true;
    ItemDataBase.push(nItem);
}

bool SaveItems()
{
    fstream fptr;
    fptr.open("items.idb", ios::binary|ios::out|ios::trunc);
    if(fptr.bad())
    return false;
    item tItem;
    while(!ItemDataBase.empty())
    {
    tItem = ItemDataBase.front();
    fptr.write((char *)(&tItem), sizeof(item));
    ItemDataBase.pop();
    }
    
    fptr.close();
    return true;
     
}
    
bool LoadItems()
{
    fstream fobj;
    fobj.open("items.idb",ios::binary|ios::in);
    if(fobj.bad())
    
    return false;
    while((fobj.eof()==false))
    {
    ItemDataBase.push(fobj.read((char *)(&item), sizeof(item) ));
    }
    fobj.close();
    return true;
    
}
};

[\source]

Sorry, I have already posted in for beginners, and noone knew what it was or suggested a solution. Feel free to ridicule, but my feelings bruse easily. 

Share this post


Link to post
Share on other sites
Plasmarobo    100
Sorry, here is the error list:
Executing make...
make.exe -f "C:\Dev-Cpp\projects\1\Makefile.win" all
g++.exe -c main.cpp -o main.o -I"lib/gcc/mingw32/3.4.2/include" -I"include/c++/3.4.2/backward" -I"include/c++/3.4.2/mingw32" -I"include/c++/3.4.2" -I"include"

In file included from main.cpp:8:
item.h: In member function `bool ItemsData::LoadItems()':
item.h:90: error: expected primary-expression before "char"
item.h:90: error: expected `)' before "char"

make.exe: *** [main.o] Error 1

Execution terminated

Share this post


Link to post
Share on other sites
Verg    450
(char *)(&item)

Shouldn't you be using a variable of type "item" instead of "&item"?

IOW

item theItem;
.
.
.
(char *)(&theItem)

Share this post


Link to post
Share on other sites
Plasmarobo    100
Okay it is now like this:


bool LoadItems()
{
fstream fobj;
fobj.open("items.idb",ios::binary|ios::in);
if(fobj.bad())
item myItem;
return false;
while((fobj.eof()==false))
{
ItemDataBase.push(fobj.read((char *)(&myItem), sizeof(item) ));
}
fobj.close();
return true;

}
};
[\source]

Compiler: Default compiler
Building Makefile: "C:\Dev-Cpp\projects\1\Makefile.win"
Executing make...
make.exe -f "C:\Dev-Cpp\projects\1\Makefile.win" all
g++.exe -c main.cpp -o main.o -I"lib/gcc/mingw32/3.4.2/include" -I"include/c++/3.4.2/backward" -I"include/c++/3.4.2/mingw32" -I"include/c++/3.4.2" -I"include"

In file included from main.cpp:8:
item.h: In member function `bool ItemsData::LoadItems()':
item.h:90: error: `myItem' undeclared (first use this function)
item.h:90: error: (Each undeclared identifier is reported only once for each function it appears in.)

make.exe: *** [main.o] Error 1

Execution terminated

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
Um... you put your item declaration inside an if statement, and to further complicate things you've taken the return false out of it!!!!


Share this post


Link to post
Share on other sites
Plasmarobo    100
Okay, thanks for catching that: New error:

Compiler: Default compiler
Building Makefile: "C:\Dev-Cpp\projects\1\Makefile.win"
Executing make...
make.exe -f "C:\Dev-Cpp\projects\1\Makefile.win" all
g++.exe -c main.cpp -o main.o -I"lib/gcc/mingw32/3.4.2/include" -I"include/c++/3.4.2/backward" -I"include/c++/3.4.2/mingw32" -I"include/c++/3.4.2" -I"include"

In file included from main.cpp:8:
item.h: In member function `bool ItemsData::LoadItems()':
item.h:91: error: no matching function for call to `std::queue<ItemsData::item, std::deque<ItemsData::item, std::allocator<ItemsData::item> > >::push(std::basic_istream<char, std::char_traits<char> >&)'
C:/Dev-Cpp/Bin/../lib/gcc/mingw32/3.4.2/../../../../include/c++/3.4.2/bits/stl_queue.h:215: note: candidates are: void std::queue<_Tp, _Sequence>::push(const typename _Sequence::value_type&) [with _Tp = ItemsData::item, _Sequence = std::deque<ItemsData::item, std::allocator<ItemsData::item> >]

make.exe: *** [main.o] Error 1

Execution terminated



code:

bool LoadItems()
{
fstream fobj;
fobj.open("items.idb",ios::binary|ios::in);
item myItem;

if(fobj.bad())
return false;
while((fobj.eof()==false))
{
ItemDataBase.push(fobj.read((char *)(&myItem), sizeof(item)));
}
fobj.close();
return true;

}
[\source]

Share this post


Link to post
Share on other sites
Roboguy    794
Quote:
Original post by discman1028
Sorry: How did he get a "[\source]" at the end of the code block, without terminating the code block at that point? :)


The terminating tag is [/source] not [\source].

Share this post


Link to post
Share on other sites
Drew_Benton    1861
Interesting code Plasmarobo, but I'd consider doing something a little different. Here's another way to do what you first posted, I added comments where approporiate for you to follow. Have a look and consider making a few changes. I added in a test so you can make a new project and work with this code by itself to get a feel for how I redesigned it. Hope you can learn some new stuff from it [smile]


//Item structure .h

// You should split this file up to a .cpp and .h style, I will leave that
// to you for it is an important programming ability.

#include <fstream>
#include <string>
#include <iostream>
// A vector is better suited for you than a queue from what I am seeing
// in this specific code. If you have to use a queue, then so be it, but I
// think a vector would be better ;)
#include <vector>

// This is fine in a .cpp file, but do not have it in a .H file
using namespace std;

// This is the structure of an item, it is seperated from the ItemsData
// because the ItemsData is a collection of this structure. You may
// need to acces this structure outside of the ItemsData class, so that's
// why I have made it global
struct tItem
{
string name;
// I made this an int because for now, there is no need to restrict
// yourself with the data of a char, feel free to change back if
// you have to
int type;
int id;
int wieght;
int value;
int para1;
int val1;
int para2;
int val2;
bool key;

// The C++ Rule of 3 is basically saying for any class, you will need
// a default ctor, a destructor, and a copy ctor. That is why I have
// added those below. There's more to the rule, but that's all you
// need to know right now

// We make a default constructor to clear out the data due to the C++ rule of 3
tItem()
{
name.clear();
type = 0;
id = -1;
wieght = 0;
value = 0;
para1 = 0;
val1 = 0;
para2 = 0;
val2 = 0;
key = 0;
}


// We make a default destructor due to the C++ rule of 3
~tItem()
{
// Nothing to do
}

// We make a copy constructor due to the C++ rule of 3
tItem(const tItem& RHS)
{
name = RHS.name;
type = RHS.type;
// Should items have a new ID if copied or do they stay the same?
// If each item should be unique, this code should change so the new
// item is generated a new ID rather than use the id of the last!
id = RHS.id;
wieght = RHS.wieght;
value = RHS.value;
para1 = RHS.para1;
val1 = RHS.val1;
para2 = RHS.para2;
val2 = RHS.val2;
key = RHS.key;
}

// For now, when you are developing your game or whatever, it is
// IMPORTANT to make sure that you know the file being written to
// files is clear, concise, and correct! Because of this, you will
// always want to save your data in text format so you can easily
// verify and validate it. When you take this approach, you can
// easily see if there is a bug in your code or in the data files
void Save(ostream& fOut)
{
fOut << name << endl;
fOut << type << " ";
fOut << id << " ";
fOut << wieght << " ";
fOut << value << " ";
fOut << para1 << " ";
fOut << val1 << " ";
fOut << para2 << " ";
fOut << val2 << " ";
fOut << key << endl;
}

// Same as what I said for the save function. When you are ready for
// deployment, you can make a new Save/Load function that will work
// in binary format. But for now, use text =)
// I also made this a bool to make sure we can load data from the file
bool Load(ifstream& fIn)
{
if(!getline(fIn, name))
return false;
fIn >> type;
fIn >> id;
fIn >> wieght;
fIn >> value;
fIn >> para1;
fIn >> val1;
fIn >> para2;
fIn >> val2;
fIn >> key;
return true;
}
};

class ItemsData
{
private:
// I made this a vector due to the lack of context for this code
// The problem right now is if you load/save items, all of the
// info is lost when you use a queue. If you use a vector, that
// data is kept and you can actually work with this class as a
// manager
vector<tItem> ItemDataBase;

public:
ItemsData()
{
// This is bad! No error checking or anything, tsk tsk =D
// Also you will not want to hard code in the file name, so
// I'd consider a reworking of this so that you call
// LoadItems yourself from outside the class rather than in
// this ctor
//LoadItems();
}

// Default dtor for this class, eralier you have protected, which
// means you would derive from this class, if that is the case,
// you will need to make this 'virtual ~ItemsData()' to properly
// allow inheritance
~ItemsData()
{
}

// No copy ctor added

// Load items based on a filename and returns true or false of the
// success.
bool LoadItems(const string& filename)
{
ifstream fobj;
fobj.open(filename.c_str());
// Need fail/is_open, not 'bad' here
if(fobj.fail() || !fobj.is_open())
return false;
bool done = false;
while(!done)
{
// This is all we have to do!
tItem temp;
done = !temp.Load(fobj);

// If the load was a success
if(!done)
{
ItemDataBase.push_back(temp);
// Since we are using extraction operators with strings,
// we have to eat endlines at the end
string buf;
getline(fobj, buf, '\n');
}
}
fobj.close();
return true;
}

// Save all of the items but keep them in the vector
bool SaveItems(const std::string filename)
{
ofstream fptr;
fptr.open(filename.c_str());
// Need fail/is_open, not 'bad' here
if(fptr.fail() || !fptr.is_open())
return false;
// This is all we have to do! Much simpler, eh?
// size_t == unsinged int (size_t is what .size() returns)
for(size_t x = 0; x < ItemDataBase.size(); x++)
{
ItemDataBase[x].Save(fptr);
}
fptr.close();
// if you wanted to remove all of the items now, all you have to do
// is call "ItemDataBase.clear();" and you have what you had before!
return true;
}

// I just added this to verify loading in game
void DisplayItems()
{
for(size_t x = 0; x < ItemDataBase.size(); x++)
{
ItemDataBase[x].Save(cout);
cout << endl;
}
}

// You only really need one of these functions because you can use
// C++'s default parameter ability. Note that when you seperate the
// prototype from the implementation for this function, the default
// parameter ONLY goes on the prototype. If you had it both places,
// then you will get a compiler error
void MakeItem(const string& iname, int iwieght, int ival, char type, bool key = false)
{
tItem nItem;
// First item will have an id of 0, second one will have 1, and so on
// If you want to start at a 1 based index, add 1 to the size
// I type cast it to int so the compiler will not complain
nItem.id = (int)ItemDataBase.size();
nItem.name = iname;
nItem.wieght = iwieght;
nItem.value = ival;
nItem.type = type;
// How the default parameter works is that the compiler will check
// this function to see if the bool key was passed when you called
// it, if it was then key contains what you had called it with.
// However, if you did not add in that parameter, the compiler
// fills it out for you with the value of false. Neat? Don't
// use this feature too much now for it leads to bad design and bugs,
// but for this example it's ok in reducing redundacy
nItem.key = key;
// Save the item to the database
ItemDataBase.push_back(nItem);
}
};

// use these defines to toggle between loading and saving quickly
// for this example
#define SAVE
//#define LOAD

int main(int argc, char* argv[])
{
ItemsData itemDB;

#ifdef LOAD
if(!itemDB.LoadItems("test.txt"))
{
cout << "Fatal Error: Items cannot be loaded" << endl;
return -1;
}
else
{
cout << "Items loaded!" << endl << endl;
// With the way I have done the tItem class, you can use
// fstream or cout to work with the data with no extra work
itemDB.DisplayItems();
}
#endif

#ifdef SAVE
// Make some random item, notice that we do not have to specify the key
itemDB.MakeItem("testItem1", 10, 65, 1);
// The previous is the same as: itemDB.MakeItem("testItem1", 10, 1, 1, false);

// Make a secpmd random item, notice this time we did use the 'key'
itemDB.MakeItem("testItem1", 10, 1, 65, true);

// Save all of the items;
if(!itemDB.SaveItems("test.txt"))
{
cout << "Fatal Error: Items cannot be saved";
return -1;
}
#endif

return 0;
}







Now that is far from perfect, but it should give you some new insight on how to go about things more efficiently. There are a few more error checks that should go in, such as making sure name is not "" when loading and stuff like that. Good luck!

Share this post


Link to post
Share on other sites
Plasmarobo    100
Thanks, that was really helpful. I just need to make it work now. I you have put me on the right track, but why can't I just inline everything? Would that make it more readible/self contained at little performace cost?

Share this post


Link to post
Share on other sites
Drew_Benton    1861
You can inline everything, but every time you change something you have to recompile since the code is literally everywhere. It's a lot easier to maintain and manage in seperate files, I think at least. As you get more and more files, somewhere down the line, you will reach a point where you might develope cyclic dependecies, and have no choice but to break up the files. Also, inlining is a complcated subject. The way you are inlining now, chances are that you are getting no performnace benefit if not a negative amount. For one read on inline, take a look at this link. There are some better articles online for it though.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this