Break My Dynamic Array Class Please?

Started by
30 comments, last by visitor 15 years ago
Ok, I think im going to rewrite almost everything, while sticking by what everyone here has said.

Thanks for you help.
Advertisement
Rewrite or refactor?
Quote:Often, inexperienced programmers will come to a point where they decide that their existing code base is fatally flawed in some way. Perhaps they’ve learned some new techniques that cannot be cleanly integrated into the code, perhaps they’ve stumbled across a requirement they cannot address, or perhaps they simply don’t like the naming conventions they used.

Whatever the reason, the programmer will often decide that the solution is a complete re-write of her code base. “This time,” she says, “I’ll get the design right the first time! My new code will be better, cleaner, more robust, more generic, and overall more powerful and less buggy!”


I could have sworn this person was writing about me specifically [lol]
I guess ill be refactoring then. Thanks again.
Quote:
I could have sworn this person

See the link in my signature ;)
Quote:Original post by jpetrie
Quote:
I could have sworn this person

See the link in my signature ;)


Whoa whoa whoa. You're the scientific ninja? Impossible!

Next you'll be telling me that they let Washu have a kid.
Mike Popoloski | Journal | SlimDX
Quote:Original post by jpetrie
Quote:
I could have sworn this person

See the link in my signature ;)


Haha, good stuff, ive already read the article on the home page but i get what you was trying to say with your last post. [smile]
I am a little confused though, in the article about Rewrite or Refactor you refer to the programmer in question as "she" so i assumed the article was written by a woman, but are you not a man? correct me if im wrong please and why would you target that specific gender ( im kinda confused here [lol] )


EDIT: Did i get rated down for this post? i thought i was at 935.. it went up from 932, to 935, and back down again to 924, i dont like this rating system i never get rated up for the all the friendly help i give to people... ( mostly because they are fairly new and have easy questions to answer lol )

[Edited by - Reegan on March 25, 2009 12:29:43 AM]
No particular reason -- why choose 'he' instead? (No, I'm not a woman.)

It's been a while since I wrote that article, so I don't recall if this was why or if it was just a whim, but generally if I have two 'people' whom I want to refer to, one of them will be male and the other female; that way the gender-specific address helps give context clues as to which of the two I'm speaking of.
Ah yeah, makes sense, thanks for clearing that up. This refactoring stuff is hard, im trying to change the code to what was suggested here but i feel like something is going to break if i refactor to much ( if you can understand what im saying ). Got any specific tips for refactoring? ill read your article on it again see if i can find any.
I'm going to chime in on the memory managment stuff, this being a game development comunity and all.
1) I have to second the "you need allocator support" comment. new/delete just don't cut it most the time on a console (sure you can wrap them up as arena allocators, but sometimes you just need an array to allocate as fixed sized blocks, or a list to allocate from a pool of small objects)
Also, look into std::vector's allocation scheme, as it is amortized O(1) for additions of unknown #'s of elements without me having to worry about manually calling "resize" with some random expected new size. Your delete function should also probably NOT resize the vector the way it does, as that is going to be very bad on performance. It should just move the elements around within the already allocated array instead of doing new->copy->delete old.

2) on your comment about "how is size_type n any easier to read than unsigned size?". Having internal typedefs for things like size, position, iterators, etc is a good thing. It lets you futureproof and idiot proof your code. size_type on a vector isn't anything special, but it COULD be a smart class that ACTS like an unsigned, but does extra checks against the vector to make sure the size meets error checking requirements. size_type could be a 32bit integer on one platform and a 64bit integer on another platform, and I, the user, don't have to care cause i can just type
vector<foo>::size_type i = 0;
and know that 'i' matches the internal implementation of vector<int>.

3) Because of your allocation policy, your implementation will have HORRIFIC!(tm) performance problems for objects with non-trivial constructors. A std::vector allocates memory SEPERATE from object construction. All your allocations as written call the default constructor on the contained class. So, in fact you are constructing objects, then assigning over the newly constructed object. This can be much much slower than copy-construction.
(look up placement new).

4) Clear() doesn't set the ArraySize to 0. This seems very fishy to me. Expecially combined with the fact that it would duplicate the functionality of Delete(). Delete has a bunch of overloads for deleting individual elements, and I'm very confused as to why Delete() actually performs a Clear(). It is a painful typo waiting to happen for someone using your code.
template <typename type>type &Array<type>::operator []( unsigned int index ) const{	// index % ArraySize = array out of bounds checking ( loops back to first index + extra baggage )	if ( ArrayData )		return ArrayData[index % ArraySize]; 	// I need to throw an exception here at some point..	return *ArrayData;}


The modulus to turn an invalid index into a "valid" one is bad form. In my experience this sort of "fool proof" style is always going to come back and bite you. I cant think of a situation where Id want to purposely pass in and index that is >= the size of an array and expect it to work.

Not asserting or throwing an exception in this case will just hide errors in the calling code from you ;)
Quote:Original post by BosskIn Soviet Russia, you STFU WITH THOSE LAME JOKES!

This topic is closed to new replies.

Advertisement