• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
irreversible

How to lose friends and alienate coworkers.

13 posts in this topic

This thread is semi-fictitious, because I don't have coworkers in my coding life. Nevertheless, I was trying to think of insidious ways to plant latent bombs in some otherwise cherished shared code (or your own, should you so desire!). Here are a few to get the ball rolling. Please feel free to post your own nefarious ideas!

 

The first two exploit the potentially single most abusive concept in coding I have in recent memory. To top things off, neither of these particular examples triggers a compiler warning in VS.

 

Do this: change your GetWidth() and/or GetHeight() functions to return uints overnight. Then get popcorn and wait for someone to do something like this:

set_screencoord(x - (GetWidth() >> 1), y - (GetHeight >> 1));

For context - I recently changed my window extent getters to uint32s. Suffice to say I'm seriously considering changing them back.

 

 

Or here's one of my favorites. Can go undetected for weeks, this one.

for(size_t i = 0; i < counter; ++i) { if(condition) { vec.quick_erase(vec.begin() + i); i--; } }

This one has gotten me a few times. The dirty bugger.

 

 

And finally something a little bit more evil. Something far more insidious, should you be naive enough to rely on simple behavior. I discovered this one in my memory manager the other day (although mine had nothing to do with clearing memory or anything that could be corruptive in any way, but rather had to do with reporting errors that led me to believe I had a serious cross-thread memory leak on my hands (which I didn't); that being said, I'd like to stress that the use of ZeroMemory() in this case is as illustrative as it gets!). Also, in my case I was dealing with smart pointers and a multithreaded memory manager, which obfuscated the issue quite a bit (although this has no real bearing on the problem at hand). What I'm trying to say is that don't judge the code - I've stripped the example down to something really really dumb and basic for reasons of clarity.

 

This one can be tough, but can you figure it out without further hints? The only assumption you should make is that allocations can be tightly packed, in that baseAddr2 = (baseAddr1 + size1 + sizeof(size_t)).

 

EDIT: a few more assumptions for the sake of clarity: assume that no (unintentional or otherwise) mistake or malicious manipulation of the allocated pointer has been made prior to calling mydelete(). Also assume that the compiler has not rearranged the members of allocrec_t - this is not the issue. Don't assume anything other than perfectly valid use of C++ in all its glory.

struct allocrec_t
{
   void* baseAddr;
   size_t size;
};
void mydelete(void * ptr)
{
   allocrec_t* ar = (allocrec_t*)ptr;

   ZeroMemory(ar->baseAddr, ar->size);
}
Edited by irreversible
0

Share this post


Link to post
Share on other sites
One way I shot myself in the foot and wasted days debugging was this:

#if defined(TESTING)
#   define protected public
#   define private public
#endif
Some libraries in the project were compiled with and without TESTING defined. Works on gcc for some reason but causes seemingly random runtime errors on Windows.
2

Share this post


Link to post
Share on other sites

This one has gotten me a few times. The dirty bugger.

struct allocrec_t
{
   void* baseAddr;
   size_t size;
};
void mydelete(void * ptr)
{
   allocrec_t* ar = (allocrec_t*)ptr;

   ZeroMemory(ar->baseAddr, ar->size);
}


OK, you win. I humble myself before you and admit my inadequacy as a programmer. You are the master and I am but a lowly hobbyist. :(

Now that the groveling is done.... What is wrong here? What am I missing? I've been banging my head against a wall for weeks!
0

Share this post


Link to post
Share on other sites

Posted (edited)

This one has gotten me a few times. The dirty bugger.

struct allocrec_t
{
   void* baseAddr;
   size_t size;
};
void mydelete(void * ptr)
{
   allocrec_t* ar = (allocrec_t*)ptr;

   ZeroMemory(ar->baseAddr, ar->size);
}


OK, you win. I humble myself before you and admit my inadequacy as a programmer. You are the master and I am but a lowly hobbyist. :(

Now that the groveling is done.... What is wrong here? What am I missing? I've been banging my head against a wall for weeks!


Not actually invoking free (depends on what that function is SUPPOSED to do, though. if it's pooling then that might be acceptable), not sanity checking ar->size (it could be uninitialized), passing void* instead of allocrec_t* bypassing any hope of type checking helping you not f*** up calls to it with the wrong type, etc, etc... Edited by Nypyren
2

Share this post


Link to post
Share on other sites

Posted (edited)

This one has gotten me a few times. The dirty bugger.

struct allocrec_t
{
   void* baseAddr;
   size_t size;
};
void mydelete(void * ptr)
{
   allocrec_t* ar = (allocrec_t*)ptr;

   ZeroMemory(ar->baseAddr, ar->size);
}


1. Unless I'm missing something, shouldn't that be something like (allocrect_t*)((unsigned char *)ptr - sizeof(allocrect_t))?
2. With the code as written, if ptr == baseAddr (which I assume it is, which is why I ask the above question) the ZeroMemory will make ar->size be 0, so if the records are tightly packed the record will show as having a 0 size - so the ACTUAL record size will be lost on deallocation, which can cause all kinds of bugs. Edited by Oberon_Command
1

Share this post


Link to post
Share on other sites

Not actually invoking free (depends on what that function is SUPPOSED to do, though. if it's pooling then that might be acceptable), not sanity checking ar->size (it could be uninitialized), passing void* instead of allocrec_t* bypassing any hope of type checking helping you not f*** up calls to it with the wrong type, etc, etc...


giphy.gif
0

Share this post


Link to post
Share on other sites

#if defined(TESTING) # define protected public # define private public #endif

 

And this is why we need a reflection API in C++^^

0

Share this post


Link to post
Share on other sites

 

#if defined(TESTING) # define protected public # define private public #endif

 

And this is why we need a reflection API in C++^^

 

Nope. There are many good reasons for (static!) reflection in C++, but this sure as hell isn't one of them. private parts of a class are NOT part of the API. protected parts are only part of the API for things that derive from it. So you should never, ever, do this.

0

Share this post


Link to post
Share on other sites

Posted (edited)

@[member='l0calh05t']

If one wants to write unit tests (whether to achieve full code coverage or not), one should include private methods as well. Except for their access modifier, they do not differ from any other methods! They have preconditions, postconditions, should be documented and ideally should be tested separetely (private methods are in fact more "unit" than public methods).

Lots of Java (or C#) codebases use reflection for this purpose:

Method method = targetClass.getDeclaredMethod(methodName, argClasses);
method.setAccessible(true);
return method.invoke(targetObject, argObjects);
Edited by matt77hias
0

Share this post


Link to post
Share on other sites

Why make it so predictable?

Random chaos is much more fun...

std::thread([]{
    while( true){
    //cause problems randomly, but only once every 15 minutes on average
    sleep(1000);
    if(0==rand()%(15*60)) {
        char *x=(void*)rand();
        //seg fault? just corrupt some data? who knows? let's make it a surprise...
        *x=rand();
    }
    }}
0

Share this post


Link to post
Share on other sites

 

@[member='l0calh05t']

If one wants to write unit tests (whether to achieve full code coverage or not), one should include private methods as well. Except for their access modifier, they do not differ from any other methods! They have preconditions, postconditions, should be documented and ideally should be tested separetely (private methods are in fact more "unit" than public methods).

Lots of Java (or C#) codebases use reflection for this purpose:

Method method = targetClass.getDeclaredMethod(methodName, argClasses);
method.setAccessible(true);
return method.invoke(targetObject, argObjects);

Nonsense. Private methods are not at all like other methods. They are implementation details! So those "pre and post conditions" are liable to change at any time, making your tests utterly worthless. A private method can even leave an object in an inconsistent state. DO NOT do this. And doing this to achieve better "code coverage" is even worse, because if any paths in private methods cannot be exercised via public methods, they are dead code and should be removed.

Or if you prefer a quote from a book, Dave Thomas and Andy Hunt write the following in Pragmatic Unit Testing:

In general, you don't want to break any encapsulation for the sake of testing (or as Mom used to say, "don't expose your privates!"). Most of the time, you should be able to test a class by exercising its public methods. If there is significant functionality that is hidden behind private or protected access, that might be a warning sign that there's another class in there struggling to get out.
0

Share this post


Link to post
Share on other sites

Posted (edited)

Nonsense. Private methods are not at all like other methods. They are implementation details! So those "pre and post conditions" are liable to change at any time, making your tests utterly worthless. A private method can even leave an object in an inconsistent state. DO NOT do this. And doing this to achieve better "code coverage" is even worse, because if any paths in private methods cannot be exercised via public methods, they are dead code and should be removed.

Your arguments make no sense. The way private methods are implemented are implementation deals like any other method. That is you encapsulate the how or the imperative nature of your code. Thus treating them as black boxes like any other method both when using them and when testing them via unit tests. Both the use and the testing make no sense without pre- and postconditions which only expose the what or the declarative nature of your code. Without having those conditions, your methods should be treated as dead code since no one would have a clue what (not how) the methods do. Saying that the conditions change can mean several things:

  1. You refactored your code. In this case, documentation, pre- and postconditions need to be updated accordingly since you modified the declarative behaviour of some methods or added some new methods.
  2. You do not understand the difference between declarative and imperative, making you leak implementation details via the documentation, pre- and/or postcondtions. In this case you should do some research on how formal documentation works.

Leaving objects in an inconsistent state is not a problem, whether this occur in a public or private method, as long as it is documented. Therefore one uses code annotations (at class, methode, parameter level, etc.) for instance in Java or just enforce immutability at the language level like Haskell.

Of course not all languages (C++, etc.) provide support for testing private methods. That is a design decision of the laguage itself and irrelevant for the general discussion. (As a sidenote: in software design research one uses languages like Haskell, First and Second Order Logic languages like IDP, since one can enforce certain pre- and/or postcondtions in the code itself (at all levels independent of some access modifier). In practice for nuclear reactors for instance, one uses different languages in their toolset as well, since one wants to control all aspects of the code.)

Finally, it is important to realize that one library of a software project has multiple APIs. You have the API you expose to the end user who is only concerned with using the code (not testing the code). The API is also exposed to API programmers which have access to all code independent of their access modifiers. The access modifier only restricts the usage range. So different API programmers may use the same private method inside the same class. If they, however, have no guarantees that the method works as intended, they could not build upon the functionality. They could detect inconsistencies by testing public methods using those private methods, but could not deduce whether the public method or private method works not as intended via the typical black box testing.

Edited by matt77hias
0

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  
Followers 0