Can I get a code review? ASE model parsing

[Kaz]

I was wondering if maybe some more experienced C++'ers could comment a little on whether the design I have here is any good or if there is some obvious thing I can do to make this better. The code is basically a series of structures designed to exactly model an ASE file, and to read one in and store it. It works fine, I'm just getting my C++ legs so I can't be certain of anything ATM . Thanks!

Header:
http://pastebin.com/ZMv9qny4

Implementation:
http://pastebin.com/KjUizxUR

[^misantropia^]

I spotted a couple of things:

1. Never use the using keyword in a header, it fouls up the global namespace of any file that includes it.

2. You should nearly always declare methods that you override as virtual (parse and print).

3. Streams are more idiomatic that printf().

4. Use an iterator to walk over a vector's elements, not at(). It makes no difference from a performance perspective but forward iterators work with all container types, at() only with a few (list, slist, vector, deque).

[Kaz]

Regarding #2 - you mean inside of the struct definitions declare them as virtual? Can you elaborate on why this is good to do?

[^misantropia^]

It's to prevent you from introducing bugs later on. Consider this example:

Code: Select all

class A {
public: 
        virtual void say() { std::cout << "A::say()\n"; }
};

class B: public A {
public: 
        void say() { std::cout << "B::say()\n"; }
};

class C: public B {
public: 
        void say() { std::cout << "C::say()\n"; }
};

int main() {
        B b; C c;
        dynamic_cast<B*>(&b)->say();
        dynamic_cast<B*>(&c)->say();
}

This works as you would expect. A::say() is virtual so this program outputs:

Code: Select all

B::say()
C::say()

But now your class hierarchy changes and B no longer inherits from A. All of a sudden the program will output:

Code: Select all

B::say()
B::say()

If you had declared your say() methods as virtual, the program would have kept working correctly. So that's why. :-)

[Kaz]

/me reads and eyes glaze over...

Thanks for the explanation. I've implemented your suggestions (at least for the most part) and done some additional things like overloading the put-to operator and using that instead of a print function, and creating parsing helper functions to scale back the number of tokenItr++ statements.

New header:
http://pastebin.com/sPGPguxn
New implementation:
http://pastebin.com/w98mwNqv

[Eraser]

Doesn't overriding non-virtual methods throw a compiler error in C++? I think the Visual Studio C# compiler does.

[Silicone_Milk]

Doesn't overriding non-virtual methods throw a compiler error in C++? I think the Visual Studio C# compiler does.

Nope.

virtual methods in C++ are overridden through the use of virtual tables which are just pointers to the most-derived available function in the inheritance tree.

For instance:

Code: Select all

class animal{
public: 
	virtual ~animal(){} 

	virtual void function_one(){ std::cout << "generic animal sound one" << std::endl; } 
	void function_two(){ std::cout << "generic animal sound two" << std::endl; } 
        
        virtual void table_example(){ /* I do nothing */ } 
};

class cat: public animal{
public: 
	~cat(){}

	virtual void function_one(){ std::cout << "meow" << std::endl; } 
	void function_two(){ std::cout << "purrrrr" << std::endl; } 
  
        virtual void table_example(){ /* I also do nothing*/ }
};

class dog : public animal{
public: 
       ~dog(){}

        virtual void function_one(){ std::cout << "woof" << std::endl; }
        void function_two(){ std::cout << "grrrrrrrrrrrrrr" << std::endl; }
}; 

There is a virtual table made for animal, cat, and dog.

The table for animal looks like:
function_pointer* a_function = &(animal::function_one);
function_pointer* b_function = &(animal::table_example);

The table for cat looks like:
function_pointer* a_function = &(cat::function_one);
function_pointer* b_function = &(cat::table_example);

And the table for dog looks like:
function_pointer* a_function = &(dog::function_one);
function_pointer* b_function = &(animal::table_example);


NOTE that because dog doesn't have a function overriding table_example() that the most-derived function available for dog is animal::table_example() yet, in the case of cat, which does provide its own version of the function, the most-derived is cat::table_example().

------------------------------------------------------------

Now if you were to create an instance of one of these classes such as:

Code: Select all

animal *kitty = new cat(); 
kitty->function_one(); //calling cat::function_one(); 
kitty->function_two(); //calling animal::function_two(); 
delete kitty; 

animal *montgomery = new dog(); 
montgomery->function_one(); //calling dog::function_one(); 
montgomery->function_two(); //calling animal::function_two(); 
delete montgomery;  

You'd get the following output:

• meow
  generic animal sound two
  woof
  generic animal sound two

Note that here the base type of the object we're calling the functions for is animal so the functions being called look like this (based off the vtable):

cat::function_one() //this IS overridden as there's an entry in cat's vtable (it's a virtual function)
animal::function_two() //this is NOT overridden since there's no entry in cat's vtable for this (it's not a virtual function)
dog::function_one()
animal::function_two()

If you created a new instance of a creature such as:

Code: Select all

cat *kitty = new cat(); 
kitty->function_one(); 
kitty->function_two(); 
delete kitty; 

Then the output would be:
meow
purrrrrr

In THIS example the objects base class is cat rather than animal. Now the functions being called look like:
cat::function_one();
cat::function_two();

-----------------------------------------

I guess, in a nutshell, an error isn't thrown because non-virtual methods don't override the inherited methods. The purpose of the virtual keyword is to say, with our example in mind, "If the function in animal also exists in the derived class, use the derived class's version." (this is function_one). Otherwise, without the virtual keyword the compiler simply uses animal's version of the function if that's the base type of the pointer used to instantiate the creature be it cat or dog. (this is function_two)

[^misantropia^]

I think Eraser is confusing C# with C++.

[Silicone_Milk]

I think Eraser is confusing C# with C++.

My novel in a sentence.

[Eraser]

I think Eraser is confusing C# with C++.

Confusing? I explicitly drew a parallel to C# in my post and asked if the worked the same?
Or do you perhaps mean to say that the virtual keyword has a different meaning in C++ as it does in C#?

edit:
after re-reading my post I realize it's a bit ambiguous.
What I really meant to ask is if your average C++ compiler doesn't throw a warning (not an error actually) when you override a non-virtual method in a derived class because that is what the C# compiler that comes with Visual Studio does when you do that sort of thing in C#.

[^misantropia^]

No, no compiler I know warns about that. I wouldn't mind it if they did but they don't. g++ has -Woverloaded-virtual but that's not quite the same thing.

Edit: and -Wnon-virtual-dtor, that one is a lifesaver.