Advice please

Mar 24, 2016 at 9:39pm
Hi,

I'm a backyard programmer so bare with me.
I've been reading a lot about anti patterns and well I've taken a long look at my code and it fits multiple examples of anti pattern.

In my code/project it fits these and other problems:

Copy paste code/ WET code (mostly in derived classes as the base class lacks the data members to properly carry it's virtual functions and literally copy paste code modified to be the other side of the coin. Also in main() the redraw logic was literally after each input check and only varied slightly based on input)
has at least one Poltergeist
Object* everywhere (I've been slowly converting them to std::unique_ptr and std::shared_ptr but only as needed)
Object orgies (most data members are public)
C style arrays (int x[50] ) everywhere
tons of global variables

I know these can all be fixed. I've been considering telling myself lesson learned and starting over with a (hopefully) better design. If you where in this situation what would you do and why?

Thanks and regards

Last edited on Mar 24, 2016 at 9:40pm
Mar 25, 2016 at 1:41am
Hi,

I am also a backyard coder, but hopefully I can mention some ideas :+)

I've been considering telling myself lesson learned and starting over with a (hopefully) better design.


Despite the current problems, does your code use normal design patterns, as in: https://en.wikipedia.org/wiki/Software_design_pattern ?

If it doesn't then a complete redesign might be warranted. I guess you would need to go through and identify where a particular pattern might improve things.

Design patterns help solve problems like excessive coupling, and having to write lots of code (or reorganise existing code) to add 1 extra thing somewhere.

In any case you would have to get rid of the problems you have identified, especially global and class public data.

Hope this helps a bit :+)
Mar 25, 2016 at 12:43pm
I would try to redesign the code. Sketch out the classes, then go through the program to see if the new design can really handle all the cases. When it can't modify the new design. Lather, rinse, repeat until you're certain that you have a new design that will work. Only then should you start writing the code.
Mar 25, 2016 at 1:10pm
Why do you actually want to fix it?
Is it not working or do you just want to learn better coding skills?
How complex is it in lines of code?
Sometimes it's easier to start again from scratch then trying to fix a mess.
Can show us the code?
Mar 25, 2016 at 8:57pm
Some very primitive UMLs:

https://www.flickr.com/photos_user.gne?path=&nsid=141279290%40N03&page=&details=1

If it doesn't work let me know

Here is the key for what those letters mean if you are curious
Key:
P = piece
Cf = formation
PL = patrol
Ex = expedition
G = gather
W = worker
M = monster
U = unit
E = element (it is the part that does the interacting, classes that don't have use an int instead)
H = hex
Gd = grid
S = settlement
B = building
D = dungeon
Sp = stockpile
I = item
T = terrain
R = resource

*1 &*2 are both poltergeist classes, they have public data members and little to zero functionality.
*3 G & W Fine example of copy and paste coding, pretty much do the same thing with respect to what they do it against)
*4 singletons most every object has a Tool*
*5 M has a
1
2
3
4
5
struct ident
	{
		monsterType type;
		monsterRace race;
	};

I'm not sure where I'm going with this but it might be something that gets moved to CF
*6 I forgot what I was going to say about this, if I remember I'll up date the post
That's the quagmire I'm working in.

The proposed new stratagem, in my opinion should, make maintaining the code easier (reducing the total lines of copy paste code)
*7 the only striking difference between CF, PL, EX and M is this std::array<std::unique_ptr<U>,someNumber>; while I'm not sure how decorators work (going to begin small scale testing after this post) I'm sure it's going to influence the number of E.

Hope this helps a bit :+)
More than you know

Why do you actually want to fix it?
To learn better coding skills

How complex is it in lines of code?
This is just a guesstamite but I'd say around 25,000, conservatively, lines (most of those lines are repeated logic)

Do you think the new model will be of use or should I keep refining it?

Last edited on Mar 25, 2016 at 9:10pm
Mar 25, 2016 at 9:40pm
I can't see anything at the link you posted.
Mar 25, 2016 at 11:45pm
I can't see anything at the link you posted.


Helps to set them to public view huh? Sorry about that!
Last edited on Mar 25, 2016 at 11:45pm
Mar 26, 2016 at 9:56am
Hi,

I had a look at your diagrams: if those are classes and inheritance, then it is looking more & more like you might need to start again, old sport :+)

OOP design is harder than what one might first imagine: it is not enough to know what a class is, then tear into writing lots of code.

There are a bunch of concepts which I am not sure whether you know about or not:

Not every class has to fit into an inheritance tree.

"IS A" relationship, implies public inheritance - have to be careful as to what this means exactly.
https://en.wikipedia.org/wiki/Liskov_substitution_principle
https://en.wikipedia.org/wiki/Open/closed_principle

"HAS A" relationship, the class has a member which is the type of another class
"USES A" relationship, a class function has another class type as an argument

Polymorphism - using virtual functions, a very powerful concept that helps write more generic code, and provide an interface.

Generic naming, as in this example:

1
2
3
4
class Animal;
class Cow : pubic Animal;
class Dog : public Animal;
class Cat : public Animal;


Each of those classes might have a Speak() function, not CowSpeak, DogSpeak, CatSpeak

The concept of "pushing things up the tree": place variables and functions as high up the tree as valid to do so (not gratuitously though), create new classes if necessary and valid. So one can define things once, and they apply to all things below it in the tree. For example: we might have Player and Enemy classes. Both have a Health value, so we can create the Actor class which Player and Enemy derive from. So now Health appears once in the Actor class.

Things such HealthPacks, Weapons, Ammo, Vehicles, Money, Tools etcetera are resources, so one might have a Resource base class.

If you are going to start again, I would recommend starting out small: 1 each of the basic things - Player, Enemy, Resource, Tool. Get that right first, then add more complexity gradually. Add in the design patterns when you see a need to decouple things, or make it easier to reuse and provide flexibility.

Some more links:

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html
http://www.dre.vanderbilt.edu/~sutambe/documents/More%20C++%20Idioms.pdf
https://isocpp.org/faq
http://gameprogrammingpatterns.com/
Topic archived. No new replies allowed.