Last day I struggled with some c-code, I decided I wanted to avoid a big switch/case, and thought I do everything in c++ instead
with a jumpTable to memberFunctions, after some struggle (since I couldn't call memberfunctions with a function-pointer from inside
the object and had to transfer all functions pointed to by the jumpTable to static functions, perhaps I'm a idiot but only solution I came up with)
I ended up with code 1000 times more unreadable than the switch/case I first had....
....then I looked at my function pointer... my god... what is this... just look at that

dwRet=(*State.pJmpTable)(&State, index, &sock, pszBuf);

Well it works, but if I drop this project for 2 days, I will look at the code with a headache
:grin:

Anybody else got some monstrous c or c++ lines of code to put in the hall of shame ?

:alright:
Posted on 2003-01-04 15:24:07 by david
David,this can happen even for asm coders.It depends on your style of thinking&
programming.Anyway,everybody has such an experience.I think it's just a matter of time
for you to find a new idea.

Good luck,

Vortex
Posted on 2003-01-04 15:37:31 by Vortex
To avoid this keep every line of code very simple. Especially in a HLL, there is no need to create unreadable code when your compiler is good. In ASM use the HLL features: STRUCT, PROC/invoke, etc. to make the code readable - don't hardcode offsets, you'll never remember what is at tomorrow. :)
Posted on 2003-01-04 16:49:52 by bitRAKE
Originally posted by david
(since I couldn't call memberfunctions with a function-pointer from inside
the object and had to transfer all functions pointed to by the jumpTable to static functions, perhaps I'm a idiot but only solution I came up with)

This seems to work fine:


class A
{
public:

void func1(int a, int b) { cout << "func1" << endl; }
void func2(int a, int b) { cout << "func2" << endl; }
void func3(int a, int b) { cout << "func3" << endl; }

void callAll()
{
for (int i=0;i<3;i++)
(this->*funcTable[i])(1,2);
}
private:

typedef void (A::*FuncPtr)(int,int);
static const FuncPtr funcTable[];
};

const A::FuncPtr A::funcTable[] =
{
A::func1,
A::func2,
A::func3
};

The only thing is that you can't store the result of call through a member pointer using the ->* or .* operator.

Thomas
Posted on 2003-01-04 17:03:44 by Thomas
Now I know why I am stuck with simple assembly :)
Posted on 2003-01-04 17:23:51 by bazik
you should learn to add spaces and then it's immediately obvious what you're doing :p
Posted on 2003-01-04 19:09:42 by Hiroshimator
Well, this is the way I do *all* switch statements that are more than 1 line of code and a break...




Class SomeObject
{

void DoSomething(int _Condition) {

switch (_Condition) {
case CONDITION_A:
DoConditionA();
break;

case CONDITION_B:
DoCondition_B();
break;

case CONDITION_AND_SO_ON:
DoCondition_And_So_On();
break;
}
}

void DoSomething_A(void) {
...
}

void DoSomething_B(void) {
...
}

Void DoSomething_And_So_On() {
...
}
}



I guess you get the general point... that the implementation of the task is not in the switch statement but in a function instead. This is useful because if there are many lines of code it'll be hard to scroll through and see what switch condition is tested and even harder if there are nested switch statements. I like it to be readable as much as possible.

Thanks,
_Shawn
Posted on 2003-01-04 23:47:05 by _Shawn
I love C++'s features... but I hate it's standard library. Any large amounts of code you create with the standard library IMHO is unreadable. That's why I use the C standard library, and C++'s OO features. In my opinion the code is much more readable, clean, crisp, and logical.
Posted on 2003-01-05 14:44:11 by Asm_Freak

I love C++'s features... but I hate it's standard library. Any large amounts of code you create with the standard library IMHO is unreadable. That's why I use the C standard library, and C++'s OO features. In my opinion the code is much more readable, clean, crisp, and logical.

Actually the standard library is the 'correct C++' way to program. It does save you from most memory management issues (new/delete) and it has some very useful containers (like (multi)maps and sorted sets). The STL is also reasonably efficient since it consists of templates which are mostly inlined in your code (iterating through an vector<int> using it's iterator produces nearly the same code as using an array pointer). For most of my programs' design I use the STL containers or my own, where speed or simplicity matters I still use arrays and other lower level stuff. Also the win32 API functions are C functions (it uses char pointers for example, not std:string's) so when dealing with the API often you can't use containers (you can't get the buffer address of a vector<char> to pass it to an API that will use it as it's buffer).

I agree with you on the readability though, in my latest project I got this error once:

X:\dev\mp3dblib\source\SROMReader.cpp(286) : error C2440: 'initializing' : cannot convert from 

'class std::_Tree<unsigned long,struct std::pair<unsigned long const ,class mp3db::Serializable *>
,struct std::map<unsigned long,class mp3db::Serializable *,struct std::less<unsigned long>,class
std::allocator<class mp3db::Serializable *> >::_Kfn,struct std::less<unsigned long>,class std::
allocator<class mp3db::Serializable *> >::const_iterator' to 'class std::_Tree<unsigned long,struct
std::pair<unsigned long const ,class mp3db::Serializable *>,struct std::map<unsigned long,class
mp3db::Serializable *,struct std::less<unsigned long>,class std::allocator<class mp3db::Serializable
*> >::_Kfn,struct std::less<unsigned long>,class std::allocator<class mp3db::Serializable *> >::iterator'
No constructor could take the source type, or constructor overload resolution was ambiguous

Huh? ;)

Thomas
Posted on 2003-01-05 15:09:15 by Thomas
:grin: Posted on 2003-01-06 23:37:57 by NaN
I agree with you on the readability though, in my latest project I got this error once:


code:--------------------------------------------------------------------------------X:\dev\mp3dblib\source\SROMReader.cpp(286) : error C2440: 'initializing' : cannot convert from
'class std::_Tree<unsigned long,struct std::pair<unsigned long const ,class mp3db::Serializable *>
,struct std::map<unsigned long,class mp3db::Serializable *,struct std::less<unsigned long>,class
std::allocator<class mp3db::Serializable *> >::_Kfn,struct std::less<unsigned long>,class std::
allocator<class mp3db::Serializable *> >::const_iterator' to 'class std::_Tree<unsigned long,struct
std::pair<unsigned long const ,class mp3db::Serializable *>,struct std::map<unsigned long,class
mp3db::Serializable *,struct std::less<unsigned long>,class std::allocator<class mp3db::Serializable
*> >::_Kfn,struct std::less<unsigned long>,class std::allocator<class mp3db::Serializable *> >::iterator'
No constructor could take the source type, or constructor overload resolution was ambiguous--------------------------------------------------------------------------------

Huh?


SACREBLEU!!!

would you mind showing the actual line(s!) of code where the error was. it looks quite intresting :grin:


To the original poster :
Why not use virual functions to accomplish what you are trying to do. Just make a base class with the function you want, and derive as many 'other' classes as you need functions.
Also in the class' constructors you can store 'this' in a vector ie:



std::vector<base*> g_myclasses;
class base
{
base() { g_myclasses.pushback(this); }
virtual ~base()
{
for( vector start to end )
{
if( (*i) == this ) g_myclasses.erase(i);
}
}

virtual void amazing();
};

class one : public base
{
void amazing();
};

class two : public base
{
void amazing();
};

for( int i = 0; i < g_myclasses.size(); i++)
g_myclasses[i]->amazing();


not sure how that would work. Maybe youd have to save the "this" pointer in each ctor for each derived class. not sure
Posted on 2003-01-25 14:21:14 by IFooBar
The line itself isn't very interesting :)

map<ulong,Serializable*>::const_iterator it = m_id2ptr.find(ptrID);


Because of the types std::map uses internally, the actual type of the iterator becomes a very long string. The actual error was just that I used iterator instead of const_iterator :grin:. The compiler could have showed the error more readable but it's hard since to the compiler, they are completely different types.

Thomas
Posted on 2003-01-25 14:26:43 by Thomas

To avoid this keep every line of code very simple. Especially in a HLL, there is no need to create unreadable code when your compiler is good. In ASM use the HLL features: STRUCT, PROC/invoke, etc. to make the code readable - don't hardcode offsets, you'll never remember what is at tomorrow. :)



Why didn't you tell me that a few months ago while I was coding my audio engine?
Posted on 2003-02-22 21:01:50 by x86asm
/* This small program (that I took the time to explain in detail, which I might add) is simply
a small password program that, if you are running Windows95/98/DOS (sorry not ME) can be added to
your autoexec.bat file to provide low-quality password protection to your computer. It simply
writes a password to a file and then compares it with the user's input and makes sure they are
equal to let you go by. Remember, I'm not a programmer yet, and if you think this code is shitty
and a complete mess, you can... well... I completely agree with you. But if you need anything, maybe more
explanations (?!), contact me at kobra@punkhardcore.net. Oh by the way, do whatever you want with
this code, you can steal it, sell it on ebay, print it and use it to wipe your ass, I don't care. I just
find it funny when people write a 5-line program and try to put a copyright on it, not realizing that
probably half the world was already using it as an example for beginners. Anyway, have fun, and I hope
you have an easier time understanding this than I did ;) */

#include <stdio.h>
#include <string.h> /* For strcmp, to compare strings */

void firstuse(void);

main()
{
FILE* file; /* Define that 'file' designates a file */
char password[9]; /* Password will be 8 characters max (null-terminated string, so gives 9), but you can make it longer and recompile it if you like */
char userinput[9];
int entrycount = 0; /* to count number of times the password was entered */
printf("Welcome to Superpass (v1.0)!\n");
file=fopen("passfile","r"); /* open a file name 'passfile' with no extension in same folder as program in read mode (r) */

if(file==0) /* if there is an error opening the file, fopen gives a value of 0 to 'file' (which is false). Often, if you do not act on an error in your program, it ends up crashing. Here we assume that an error means the non-existence of 'passfile' and thus its first use. */
{
firstuse(); /* call firstuse */
file=fopen("passfile","r"); /* reopen the file in read mode for password checking */
}
else /* i.e. if the file exists */
printf("Please enter your password:\n(in the event of a mistake, press ENTER to re-enter the password)...\n");
scanf("%s",&userinput); /* user input for first enter only */
fgets(password,9,file); /* get the password (8 first characters of the file) from the file */
while(strcmp(password,userinput)!=0) /* use strcmp to compare the password from the file and the user's input. if they are the same, the difference will be of zero, thus skipping the 'while' sequence. Anything else than 0 indicates a difference in the passwords */
{
entrycount++; /* increase 'entrycount' by 1, to add to the number of times the user tried to enter the password */
while(entrycount >= 3) /* after the third user try, there is no more chances. Thus, if 'entrycount' is equal or bigger (just to be safe, but you could put simply 'equal') than 3, the program then writes 'Intruder Alert!' on the screen endlessly until the computer crashes. Very horrible coding, I know, but it will have to do for now :) */
printf("\aINTRUDER ALERT!\n");
printf("\aWrong password! Please try again...\n"); /* Unless 'entrycount' reaches 3, each time the user makes a mistake, display this */
scanf("%s",&userinput); /* for every try the program has to rescan the user input and put it in 'userinput' */
}

printf("Thank you for using Superpass. Please continue...\n"); /* if the password was the same as the one in the file, skip the whole 'while' bogus, display the message, close the file and return nothing. That's it. */
fclose(file);
return 0;
}

void firstuse()
{
FILE* file; /* we'll just use the same one here ('file'), we only need one since there are no real values to keep in it (we might also want to get rid of that 0 value in 'file' - though it probably wouldn't do much anyway) */
char firstinput[9];
printf("Please enter the password you would like to use with Superpass.\nThe password can be up to 8 characters long...\n"); /* note that the password can also be shorter */
scanf("%s",&firstinput); /* get user input and put the string in 'firstinput' */
file=fopen("passfile","w"); /* open/create 'passfile' in write mode (w). This mode is destructive, so if the file has something in it (though at this point it shouldn't, unless our error is of an other origine somehow) it is erased. It also creates the file if it doesn't exist. */
fprintf(file,"%s",firstinput); /* print into the file a string, found in 'firstinput' */
fclose(file); /* close file so the data inside isn't damaged or lost */
printf("\nYour password has been saved. Please re-enter it...\n");
}

/* Note: I used the Digital Mars compiler v827.5 for this www.digitalmars.com */

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

that was my first and only C program. i wrote so many comments it started to overcrowd the code. then i switched to assembly.
Posted on 2003-03-13 13:37:54 by Kobra
c is polluting your mind :grin: Asm coders need no comments. Seriously I code without much of a comment.
Posted on 2003-03-14 05:49:07 by roticv
But if you gonna use comments read this, and then take an afination :DThe Art of Code Documentation (and commentation)

also in the future you gona see that you only need clarificate some things, orexplain the complex read, i think it will be from use.

take the control, is only a guide you will use or not, and make your own

Have a nice day.
Posted on 2003-03-14 13:08:28 by rea
thanks i guess
Posted on 2003-03-14 13:26:32 by Kobra
line-by-line commenting isn't too smart. comment "code blocks". switching to asm because you haven't developed a proper commenting style? seems a bit silly ;).

I'm going to duplicate my current signature here, as - who knows - I might change it sometime in the future:
"Any fool can write code that a computer can understand. Good programmers write code that humans can understand."
Posted on 2003-03-31 01:56:05 by f0dder

"Any fool can write code that a computer can understand. Good programmers write code that humans can understand."


"Bad anti-crackers write code that humans can understand." :grin:

(i hope that's an acceptable comment...)
Posted on 2003-03-31 13:45:00 by jademtech
this monster in vb

For n = 1 To BackDrops

If BackDrop(n).Visible = True And BackDrops > 0 Then
If BackDrop(n).VisLevel = 255 Then
TransparentBlt Me.hdc, BackDrop(n).X, BackDrop(n).y, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, Graphics.hdc, Frame(BackDrop(n).Frame).X1, Frame(BackDrop(n).Frame).Y1, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, Frame(BackDrop(n).Frame).TransparentColor
Else
If BackDrop(n).VisLevel <> 0 Then
BitBlt TransBuffer.hdc, 0, 0, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, Me.hdc, BackDrop(n).X, BackDrop(n).y, vbSrcCopy
TransparentBlt TransBuffer.hdc, 0, 0, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, Graphics.hdc, Frame(BackDrop(n).Frame).X1, Frame(BackDrop(n).Frame).Y1, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, Frame(BackDrop(n).Frame).TransparentColor
TransBlt Me.hdc, BackDrop(n).X, BackDrop(n).y, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, TransBuffer.hdc, 0, 0, Frame(BackDrop(n).Frame).Width, Frame(BackDrop(n).Frame).Height, BackDrop(n).VisLevel
End If
End If
End If
Next n


this were like 6 lines of code
only the forum clipped them
Posted on 2005-01-07 17:25:12 by Warsocket