Quake3World.com Forums
     Programming Discussion
        Can I get a code review? ASE model parsing
Post new topicReply to topic
Login | Profile | | FAQ | Search | IRC




Print view Previous topic | Next topic 
Topic Starter Topic: Can I get a code review? ASE model parsing
Kaz
Señor Shambler
Señor Shambler
Joined: 07 Mar 2006
Posts: 849
PostPosted: 05-27-2011 08:26 AM           Profile Send private message  E-mail  Edit post Reply with quote


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 :p. Thanks!

Header:
http://pastebin.com/ZMv9qny4

Implementation:
http://pastebin.com/KjUizxUR
Top
                 
^misantropia^
Mentor
Mentor
Joined: 12 Mar 2005
Posts: 3958
PostPosted: 05-27-2011 09:15 AM           Profile Send private message  E-mail  Edit post Reply with quote


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).
Top
                 
Kaz
Señor Shambler
Señor Shambler
Joined: 07 Mar 2006
Posts: 849
PostPosted: 05-27-2011 09:43 AM           Profile Send private message  E-mail  Edit post Reply with quote


Regarding #2 - you mean inside of the struct definitions declare them as virtual? Can you elaborate on why this is good to do?
Top
                 
^misantropia^
Mentor
Mentor
Joined: 12 Mar 2005
Posts: 3958
PostPosted: 05-27-2011 01:30 PM           Profile Send private message  E-mail  Edit post Reply with quote


It's to prevent you from introducing bugs later on. Consider this example:
Code:
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:
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:
B::say()
B::say()

If you had declared your say() methods as virtual, the program would have kept working correctly. So that's why. :-)
Top
                 
Kaz
Señor Shambler
Señor Shambler
Joined: 07 Mar 2006
Posts: 849
PostPosted: 05-27-2011 04:15 PM           Profile Send private message  E-mail  Edit post Reply with quote


/me reads and eyes glaze over... :p

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
Top
                 
Eraser
Cool #9
Cool #9
Joined: 01 Dec 2000
Posts: 44136
PostPosted: 05-28-2011 11:24 PM           Profile   Send private message  E-mail  Edit post Reply with quote


Doesn't overriding non-virtual methods throw a compiler error in C++? I think the Visual Studio C# compiler does.
Top
                 
Silicone_Milk
Immortal
Immortal
Joined: 12 Mar 2005
Posts: 2205
PostPosted: 05-29-2011 12:42 AM           Profile   Send private message  E-mail  Edit post Reply with quote


Eraser wrote:
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:
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:
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:
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)




Last edited by Silicone_Milk on 08-01-2011 08:21 AM, edited 1 time in total.
Top
                 
^misantropia^
Mentor
Mentor
Joined: 12 Mar 2005
Posts: 3958
PostPosted: 05-29-2011 02:39 AM           Profile Send private message  E-mail  Edit post Reply with quote


I think Eraser is confusing C# with C++.
Top
                 
Silicone_Milk
Immortal
Immortal
Joined: 12 Mar 2005
Posts: 2205
PostPosted: 05-29-2011 02:47 AM           Profile   Send private message  E-mail  Edit post Reply with quote


^misantropia^ wrote:
I think Eraser is confusing C# with C++.


My novel in a sentence. :tard:
Top
                 
Eraser
Cool #9
Cool #9
Joined: 01 Dec 2000
Posts: 44136
PostPosted: 05-29-2011 11:01 AM           Profile   Send private message  E-mail  Edit post Reply with quote


^misantropia^ wrote:
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#.
Top
                 
^misantropia^
Mentor
Mentor
Joined: 12 Mar 2005
Posts: 3958
PostPosted: 05-29-2011 02:23 PM           Profile Send private message  E-mail  Edit post Reply with quote


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.
Top
                 
Quake3World.com | Forum Index | Programming Discussion


Post new topic Reply to topic


cron
Quake3World.com
© ZeniMax. Zenimax, QUAKE III ARENA, Id Software and associated trademarks are trademarks of the ZeniMax group of companies. All rights reserved.
This is an unofficial fan website without any affiliation with or endorsement by ZeniMax.
All views and opinions expressed are those of the author.