Frequently encountered errors

[TOMNKZ]

Hi
I want to know what errors do you meet often in not yours code,in code of beginner. Especially, code which works, but still bad?:o

[aio]

Code errors on codes that works?

You probably mean general problems with a beginner's codes because if it works, it has no error.

I have little experience evaluating other beginner's codes. I can only recall that when I compared my codes lately with my codes when I was a beginner, I often find something like:

- There are variable assignments that can be placed strategically to make progam more efficient. Example, I sometimes assign values inside a loop when that value does not necessarily change on the duration of said loop.

- Because I cannot memorize all functions, there are instances when I create one not knowing that it's already available.

- I have tendencies to use variable type larger than necessary (in VB - Long instead of integer) because I felt then they're "much safer."

- sometimes I defined dead variables or even dead functions.

To sum it all, my common "error" (if you put it that way) is that my beginner's codes are often less efficient -- but they have worked.

[TOMNKZ]

I mean errors which reduce effectiveness of code
or in future during redesign or some modifications cause errors
or make something more complicated than it is.

[Andy Tacker]

Mushroom programming is the biggest flaw developers make when coding. some people call it serail programming... that means..

my function()
{
dothisaction
if(doseconaction)
dothirdaction
updatethis
updatethat
openfileorregistery
saveinformation
Thankyoujobfinished
}

this is what OOP does not teach :) avoid this kind of work...

be sure to minimize exception raisings... use try catch or check errors...

never hardcode functionalities. try to be open. keep space for further enhancements of the code...

what else? nothing much really... efficiency is not measured in LINES of code :)

avoid my suggestions offcourse :p

[Alsvha]

I mean errors which reduce effectiveness of code
or in future during redesign or some modifications cause errors
or make something more complicated than it is.

Things like ....

Accidently keeping references to objects that are no longer needed :)
Automatic garbage collection is nice .... if you remember to cut the references to the objects :o

Removing references to objects which are needed again later in the code, so one creates the same object several times.

[Norfy]

I hate finding this in (not necessarily) beginners code:
bool function()
{
if (<bool expression> == true)
{
return true;
}
else
{
return false;
}
}N.B. I'm not referring to languages where bool is not an actual type.

[cloureir]

This is what I've found...
these are not exactly errors in *code* :
-forget to commit updates in DB.
-forget to insert comments in code to make other coders' life easier.
-do not follow coding standards
-In GUI - forget about TAB ORDER of objects on sreens...as they (beginners) usually use mouse they forget about the users who prefer to use keyboards to enter data.
-In GUI they type messages that are not understood by users.... like for example, Operation Commited, instead of, for example, Application Sumitted.

That's all I can think for now.
:wave:

[aio]

This is what I've found...
...
-In GUI - forget about TAB ORDER of objects on sreens...as they (beginners) usually use mouse they forget about the users who prefer to use keyboards to enter data.
...
:wave:
I totally agree with this one. Most beginner thinks that all users depended so much on mouse. I have encountered several of these on some of my newbie office mates before.

And if I may add, there are lot's of tendencies newbies would overkill the design of the forms(like fonts too big, and multi-colored which in most instances are rather nuisance than helpful).

[ovidiucucu]

Hi
I want to know what errors do you meet often in not yours code,in code of beginner. Especially, code which works, but still bad?:o
I have to add here that can exist bad code which still works but not forever. And this is the most dangerous because usually it crashes after the testing phase at the customer.
For example:

too large Cyclomatic complexity (http://msquaredtechnologies.com/m2rsm/docs/reports/rsm_c_report.htm)
allocate something without freeing (memory, handles, etc)

[ovidiucucu]

For C/C++/Java you can use this analysis tool (http://www.fileheaven.com/Resource-Standard-Metrics-for-C-C++-and-Java/download/15323.htm) to detect bad style issues in source code.

[cloureir]

Years ago I saw this:
"Delete From Table_XXX"
that's it.... no Where clause!!!!! :eek:
Fortunately the DBA restored the data.

[ovidiucucu]

These also can generate trouble in a program sooner or later:
SELECT * FROM blahblah
INSERT INTO blahblah VALUES(...)

[ovidiucucu]

This stupid one made computer frozen (at the client of course ;)):
..... WHERE ...<some good joins here>... AND a='A' OR a='B' OR a='C'
instead of:
..... WHERE ...<some good joins here>... AND a IN('A','B','C')

[ovidiucucu]

Returning to C/C++
This one makes me often feeling sorry I have not a machine gun:
HRESULT hr = SomeFunction();
_ASSERTE(SUCCEEDED(hr));

[cilu]

Here's one that makes me wish a had a gun:

byte _AnalogProperties[8]; // no initialization here

if(<condition>) // this one can be false
{
// perhaps a loop here
_AnalogProperties[some_index] = some_value;
}

SetTheseProperties(_AnalogProperties, sizeof(_AnalogProperties));

[ovidiucucu]

if(TRUE == SomeFunctionReturningBOOL())
{
//... do something
}
Here is a nice one:
if(<some_error_condition>)
{
AfxMessageBox(strError.LoadString(IDS_SOME_ERROR_MESSAGE));
}

[Krzemo]

General Database programming:

Begin transaction.
Do some work
if(error)
{
Message box with error description (or not:eek: )
rollback transaction -(and all other users wait for user to click "OK", but user goes to administrator for help :D )
}
else commit transaction

[Firewireguy]

Ones I hate are inconsistency:

iChargingFreq = GetPaymentFreq();


where the payment frequency may be totally different to the charging frequency yet the variable names used makes the code almost impossible to understand.

not technically an "error", but I wouldn't pass it in any code review.

[Krzemo]

int *pArray= new int [x];
delete pArray;:D

[ovidiucucu]

Is somebody curious to push <Yes> ?

[cloureir]

I hope you enjoy this one. Huge but it works! :cool:
Problem is, it's difficult to maintain. :(

SELECT a.num_lev_modu,
a.co_crc_modu,
A.co_modu,
A.na_cmp_modu,
A.na_abr_modu,
A.num_CR, A.co_depc_adm,
A.co_area_acd,
A.co_dptm_acd,
A.co_lect_prin,
A.co_sect,
A.qt_vact_disp,
A.num_grp_sect,
SUBSTR( VALUE(D.na_last_pers,' ')|| ' ' || VALUE(D.na_mid_pers,' ')|| ' ' ||

VALUE(D.na_first_pers,' '),1,30),
a.fg_impr_ttb,
a.fg_stat_sect,
a.fg_stat_prog_sect,
a.qt_stu_regi,
a.fg_lock_regi,
a.fh_last_regi,
a.qt_vact,
A.fg_vact_sect,
A.fg_ctl_vact_sect,
CAPC_ambi

FROM (
SELECT A.num_lev_modu,
A.co_crc_modu,

B.co_modu,
B.na_cmp_modu,
B.na_abr_modu,
B.num_CR,
B.co_actv_lect_prin,
B.co_area_acd,
B.co_dptm_acd,

C.AA_semt,
C.num_semt,
C.num_sec_semt,
C.co_depc_adm_sect as co_depc_adm,
C.co_lect_prin,
N.co_sect,
C.fg_regi_grp_sect,
C.fg_grp_sect,
C.fg_stat_prog_sect,
C.fg_lock_regi,
C.fg_vact_sect,
C.fg_ctl_vact_sect,
0 AS num_grp_sect,
D.qt_vact,
D.qt_stu_regi,
(D.qt_vact - D.qt_stu_regi) AS qt_vact_disp,
C.fg_impr_ttb,
C.fg_stat_sect,
C.fh_last_regi,

(Select MIN(w.qt_capc_pers) AS qt_max_pers
from db2host.activity_lect x,
db2host.type_activ_lect y,
db2host.timetable_activ_lect z,
db2host.room w
where x.co_actv_lect = y.co_actv_lect and
x.aa_semt = C.aa_semt and
x.num_semt = C.num_semt and
x.co_modu = C.co_modu and
x.co_sect = C.co_sect and
y.co_grp_actv_lect = 1 and
y.co_Sgrp_actv_lect = 1 and
x.num_actv_lect = z.num_actv_lect and
(z.num_grp_sect is null or
z.num_grp_sect = 0) and
z.date_sta_vig_ttb <= date('1997-09-01') and
z.date_end_vig_ttb >= date('1999-08-31') and
w.num_ambi = z.num_ambi) AS CAPC_ambi

FROM db2host.plan A
inner join db2host.module B
on a.co_modu = b.co_modu AND
a.co_espc = 'CS76000' and
a.num_lev_modu >= 1 and
a.num_semt_end_VIG is NULL

INNER JOIN db2host.program B1
ON A.co_espc = B1.co_espc


inner join db2host.section N
ON A.co_modu = N.co_modu AND
N.AA_semt = 2005 AND
N.num_semt = 1 AND
N.num_sec_semt = 1 AND
N.co_sect/100 = A.num_lev_modu AND
N.co_modu_ref IS NOT nuLL AND
N.co_sect_ref IS NOT nuLL

inner join db2host.section C
ON N.co_modu_ref = C.co_modu AND
N.AA_semt = C.AA_semt AND
N.num_semt = C.num_semt AND
N.num_sec_semt = C.num_sec_semt AND
N.co_sect_ref = C.co_sect AND
C.fg_sect_adms = '0' AND
C.fg_ctl_vact_sect = '1' AND
C.fg_stat_sect = '0' and
C.fg_stat_prog_sect = '1'

INNER JOIN db2host.Department C1
ON C1.co_depc = C.co_depc_adm_sect AND
B1.co_depc = C1.co_depc_sup

INNER JOIN db2host.places_section D
ON C.AA_semt = D.AA_semt AND
C.num_semt = D.num_semt AND
C.num_sec_semt = D.num_sec_semt AND
C.co_modu = D.co_modu AND
C.co_sect = D.co_sect AND
C.co_modu = D.co_modu AND
D.co_espc = A.co_espc

UNION

SELECT A.num_lev_modu,
A.co_crc_modu,

B.co_modu,
B.na_cmp_modu,
B.na_abr_modu,
B.num_cr,
B.co_actv_lect_prin,
B.co_area_acd,
B.co_dptm_acd,

C.AA_semt,
C.num_semt,
C.num_sec_semt,
C.co_depc_adm_sect AS co_depc_adm,


(Select max(w.co_lect)
from db2host.activity_lect x,
db2host.timetable_activ_lect y,
db2host.timetable_labor_lect z,
db2host.labor_lect_assig w

where x.aa_semt = c.aa_semt and
x.num_semt = c.num_semt and
x.co_modu = c.co_modu and
x.co_sect = c.co_sect and

y.num_actv_lect = x.num_actv_lect and
y.num_grp_sect = c2.num_grp_sect and
y.date_sta_vig_ttb <= date('1997-09-01') and
y.date_end_vig_ttb >= date('1999-08-31') and

z.num_ttb = y.num_ttb and
z.date_sta_vig_ttb <= date('1997-09-01') and
z.date_end_vig_ttb >= date('1999-08-31') and

w.num_lbor = z.num_lbor) as co_lect_prin,

N.co_sect,
C.fg_regi_grp_sect,
C.fg_grp_sect,
C.fg_stat_prog_sect,
C.fg_lock_regi,
C.fg_vact_sect,
C.fg_ctl_vact_sect,
C2.num_grp_sect,
C2.qt_vact,
C2.qt_stu_regi,
(C2.qt_vact - C2.qt_stu_regi) AS qt_vact_disp,
C2.fg_impr_ttb,
C2.fg_stat_grp_sect AS fg_stat_sect,
C2.fh_last_regi,

(Select MIN(w.qt_capc_pers) AS qt_max_pers
from db2host.activity_lect x,
db2host.type_activ_lect y,
db2host.timetable_activ_lect z,
db2host.room w
where x.co_actv_lect = y.co_actv_lect and
x.aa_semt = C.aa_semt and
x.num_semt = C.num_semt and
x.co_modu = C.co_modu and
x.co_sect = C.co_sect and
y.co_grp_actv_lect = 1 and
y.co_Sgrp_actv_lect = 1 and
x.num_actv_lect = z.num_actv_lect and
z.num_grp_sect = C2.num_grp_sect AND
z.date_sta_vig_ttb <= date('1997-09-01') and
z.date_end_vig_ttb >= date('1999-08-31') and
w.num_ambi = z.num_ambi) AS CAPC_ambi


FROM db2host.plan A
inner join db2host.module B
on A.co_modu = B.co_modu AND
a.co_espc = 'CS76000' and
A.num_lev_modu >= 3 and
A.num_semt_end_VIG is NULL)

INNER JOIN db2host.program B1
ON A.co_espc = B1.co_espc

inner join db2host.section N
ON N.co_modu = A.co_modu AND
N.AA_semt = 2005 AND
N.num_semt = 1 AND
N.num_sec_semt = 1 AND
N.co_sect/100 = A.num_lev_modu AND
N.co_modu_ref IS NOT NULL AND
N.co_sect_ref IS NOT NULL

INNER JOIN db2host.group_section C2
ON C2.co_modu = N.co_modu_ref AND
C2.AA_semt = N.AA_semt AND
C2.num_semt = N.num_semt AND
C2.num_sec_semt = N.num_sec_semt AND
C2.co_sect = N.co_sect_ref AND
C2.fg_stat_grp_sect = '1'

inner join db2host.section C
ON C.co_modu = C2.co_modu AND
C.AA_semt = C2.AA_semt AND
C.num_semt = C2.num_semt AND
C.num_sec_semt = C2.num_sec_semt AND
C.co_sect = C2.co_sect AND
C.fg_stat_sect = '0' and
C.fg_stat_prog_sect = '1'

INNER JOIN db2host.department C1
ON C1.co_depc = C.co_depc_adm_sect AND
B1.co_depc = C1.co_depc_sup

) A

LEFT OUTER JOIN db2host.employee C
ON A.co_lect_prin = C.co_empl

LEFT OUTER JOIN db2host.persON D
ON C.co_pers = D.co_pers


ORDER BY a.num_lev_modu, A.co_modu, A.co_sect, A.num_grp_sect

[ovidiucucu]

Yeah, I remember such kind of queries generally made by some wise analysts using a visual tool. What's bad is that almost always if something goes wrong it's a developers' task to find out why.

[ovidiucucu]

THE EMPTY TABLE BUG
A wise programmer expects that a query returns at lest one row, so why wasting time to test it?
Well, must happen, usually at the client and the aplication creshes before a coherent prompt message.

No problem, if it's running under XP, you can tell the client to push "Send Error Report" (see attached).
Maybe MS can find a solution... :D :D

[ovidiucucu]

A fresh one
void CFoo::Init(int x, int y)
{
if(!m_db.Open())
return;

// in some circumstances nice, good, appetitive garbage below
m_x = x;
m_y = y;
}
/yummy, yummy :D

[Yves M]

The one I really like:

if (bCondition && bOtherCondition) {
Dosomething();
if (!bCondition) {
// error message
}
}

Or the eternal:

If Condition And c="a" Or c="b" Or c="c" Then
' Do something
End If

[ovidiucucu]

namespace
{
BOOL LOOK_IN_TRACER = FALSE;
}
// ...
// ...
// ...
if(m_odtBegDate > m_odtEndDate)
{
ERROR_TRACE(_T("beginning date should be smaller than or equal to ending date"));
_ASSERTE(LOOK_IN_TRACER);
}
// ...
// ...
// ...

Please, can somebody give me a gun?!

[darwen]

I suppose my major gripe is about completely pointless comments.

I see this kind of thing all the time, especially from people who shout about how great auto-documentation software is. Makes me laugh.


// class which does the drawing
class DrawingClass
{
public:

// this method draws a circle
void DrawCircle();
}


Also


// this is the counter for the number of oranges
int nX = 0;

// why not just do :
int nOrangesCount = 0;


Another of my pet hates is people typedef-ing STL classes. This makes the code exceptionally unreadable.


// somewhere in a .h
typedef std::vector<int> INTVECTOR;

void MyFunction()
{
// what the heck is INTVECTOR ???
INTVECTOR vector;
}


Of course all the standard ones stand : like 1000 line methods with huge blocks of commented out code in them.

Not using enums for constants as well. What the heck is '10276' when it's at home ? (or any other constant).

Man, I could go on forever.

Darwen.

[RoboTact]

(about Java code)

There is a "rule": when class grows more then (even more so) 2000 lines, you should cut the problem to smaller entities. Guess what? New class is created (say, Derived) subclassing original one (say, Base) and new methods just appear in Derived instead of Base, every now and then being accessed from Base using ((Derived)this).theNewMethod() conversion.

Class fields are evidently all public and are accessed and being changed (of course they are mutable) from everywhere. Variables in procedures are listed at the beginning of procedure, named by pattern:Var var;
Var varr;
Var varrr;
Var var2;

[ovidiucucu]

Man, I could go on forever.

Darwen.
Be my guest. :D ;) :thumb:

[enfekted]

My pet peeve is when a developer always throws the same exception from every method they write making it impossible to catch specific exceptions being thrown. Or worse, rethrowing a less informative exception in the place of a helpful exception.


public class Widget
{
public void ConnectToDatabase()
{
try
{
odbc.Open()
ocbc.ExceuteQuery()

/* Do a bunch of other stuff */

odbc.Close()
}
catch ( OdbcException ex )
{
throw new Exception( "Something went wrong" );

// -- OR --

MessageBox.Show( "Something went wrong" );
}
}
}


And then I go to execute this method and get back a general exception that "Something went wrong". And then I have to guess what that "something" was...

I can't tell you how many times I've come across this or something like it

[Skoons]

You will never belive but some years ago I have found something like this in project which was on last test before release :eek:

p=malloc(bytes);
if(p==NULL)
{
ShowMessage("Can`t allocate enough memory");
exit(EXIT_FAILURE);
}


So this software (it was written on C language as you see) if can`t find some more bytes for new data object closed itself, and even not ask user to save his work.

[xufeisjtu]

Returning to C/C++
This one makes me often feeling sorry I have not a machine gun:
HRESULT hr = SomeFunction();
_ASSERTE(SUCCEEDED(hr));
what's the code you suggest, then?
I'm a beginner...

[ovidiucucu]

what's the code you suggest, then?
I'm a beginner...
Well, that is an example about how/where to not use _ASSERT, _ASSERTE Macros (http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt__assert.2c_._asserte_macros.asp).
They are very good to show at debug time that a programming mistake is made.
But in example above it uses a success/failure return value of a function. Usually that function fails at the client ;) (in release buid) and the application crashes giving no one sigificant info why.

[ovidiucucu]

I suppose my major gripe is about completely pointless comments.
I love the code in that nobody get tired to delete nice'n useful comments like
// TODO: Add your specialized code here and/or call the base class
or
return TRUE; // return TRUE unless you set the focus to a control
// EXCEPTION: OCX Property Pages should return FALSE
:D

[darwen]

How about classes which aren't named in entirity but are named in SMS-speak ?


class CMFxTVp
{
}


Copied and pasted code all over the place too...

How about the classic "I've discovered the mutable keyword" one :


class CMyClass
{
public:
void SetA(int nValue) const { m_nA = nValue; }
void SetB(int nValue) const { m_nB = nValue; }

private:
mutable int m_nA;
mutable int m_nB;
} ;


Don't laugh : I've actually seen this one ! From a contractor too !

How about curly bracket blocks for no reason at all in methods ? (I'm not talking about having curly bracket blocks containing an object which has to be destroyed at end of the block).

A great one in C# is people using Get and Set methods instead of properties because "they don't like properties." Yeah, until you have to do something using reflection : like XmlSerialization : then you're stuffed boyo aren't you ?

Darwen.

[ovidiucucu]

Dear programmers,

Never forget to show a message box in WM_TIMER message handler functions, to signal whenever an error occurs:
Like in this example:
// ...
SetTimer(0, 100, NULL);
// ...
void CFoo::OnTimer(UINT nIDEvent)
{
switch(nIDEvent)
{
case 0:
if(! GetSomething())
{
AfxMessageBox(_T("Something was not found"));
}
break;
// ...
}
}
A little improvement: instead using AfxMessageBox, create a modeless dialog in a random position; in this way the bored user can have sometimes a nice surprise and enjoy a game, very similar with the popular Kill The Popups (http://www2.b3ta.com/realistic-internet-simulator/).

;)

[ovidiucucu]

About using of HRESULT again:
HRESULT CFoo::ABigFunction()
{
// ...
if(...some error condition here...)
return E_FAIL; // failed

// ...

if(...another error condition here...)
return E_FAIL; // failed

// many other similar returns...

return S_OK;
}
// ... somewhere in another function
HRESULT hr = ABigFunction();
if(FAILED(hr))
{
CString strError;
strError.Format(_T("%08X"), hr);
AfxMessageBox(strError, MB_ICONERROR);
// An excellent opportunity for the end user to find the value of E_FAIL.
// No comment about the maintenance guy feelings...
// (just he has received the screenshot below).
}

[Skoons]

About using of HRESULT again:...

Yeah, even if you are skilled programmer, you never tell where problem is :D .
Better idea to tell something like - "Can`t open file [FILEPATH]", or something what describes function with error and where it is. But as we know in many OS`s there are many messages like youre example

[RoboTact]

No, really error doesn't need to be displayed in spellable form, there should only be the way to give developer enough hint about where the problem is. In many cases source line number/IP is enough. But it does matter that programm should have way to report error in any case (at least on debug mode) and way to recover in release mode. It's way too often when something is written like this:void doSomething(){
if(thisIsFine() && thatIsFine())
{
// actually do that something
}
// no error reporting
}or like this:void ShowThatLittleDotInTheCorner()
{
if(somethingWentWrong())
TerminateWholeApplicationWithoutAnyReport();
}

[ovidiucucu]

But as we know in many OS`s there are many messages like youre example
Here is nice one I saw long time ago (well, I have to recognize this is a meaningful one :D).

[ovidiucucu]

[/code]or like this:void ShowThatLittleDotInTheCorner()
{
if(somethingWentWrong())
TerminateWholeApplicationWithoutAnyReport();
}
Yeah, that's good to prevent catastrophic events when the application becomes crazy (e.g. refusing to show little dots).
I will insist for everybody in next code inspection session to fullly adopt this good, healthy approach. :D ;)

[SuperKoko]

- I have tendencies to use variable type larger than necessary (in VB - Long instead of integer) because I felt then they're "much safer."


Except if this data is in a big array and size matters, Long (on 32 bits platforms) is always preferable to Integer.
It can contains much more different integers.
And it is FASTER.
Because 32 bits processors need a "16 bits operands prefix" to perform 16 bits operations.
It increases the code size (but reduces the data size :) ).
And it needs more CPU cycles to be computed by the CPU.


Mushroom programming is the biggest flaw developers make when coding. some people call it serail programming... that means..
Code:

my function() { dothisaction if(doseconaction) dothirdaction updatethis updatethat openfileorregistery saveinformation Thankyoujobfinished }



this is what OOP does not teach avoid this kind of work...

Even, good old procedural programming (C, Fortran, BCPL...) teach to avoid that!


what else? nothing much really... efficiency is not measured in LINES of code


There are many guys who think that:

something && something_else;

Is faster than

if (something) something_else;


And the worst thing is that, "something && something_else" may be slower with some very-bad compilers (and embedded platforms often have bad compilers).

These same guys tend to write code with undefined behavior, such as "array[i]=i++;"



I hate finding this in (not necessarily) beginners code:
Code:

bool function() { if (<bool expression> == true) { return true; } else { return false; } }

N.B. I'm not referring to languages where bool is not an actual type.

I found code like that in an STL implementation (I conceal the name)!
In the same STL I found also many obviously suboptimal codes where trivial naive implementations are better:

template <class InputIterator, class T>
Function for_each (InputIterator first, InputIterator last, Function f)
{
while (first != last) f(*first++);
return f;
}


It uses first++ instead of ++first

[darwen]

From RoboTact :


No, really error doesn't need to be displayed in spellable form, there should only be the way to give developer enough hint about where the problem is


I can only say, I'm glad I don't use any of your software - I'd hate to be a user, be working along quite nicely and have a message box with "error 0x8020304" come up and my last 6 hours work be wiped out for no reason.

I'm amazed that someone actually thinks that reporting an 0x error is acceptable.

I bet you use STL and have huge memory leaks in your code you can't be bothered fixing too. And even if you wanted to fix the memory leaks, you wouldn't be able to - using STL and all.

Darwen.

[klintan]

Of course all the standard ones stand : like 1000 line methods with huge blocks of commented out code in them.


This one I hate too. Especially when a version control system is used (and I cannot think of a reason not to use a version control system).

The last 7-8 years I have been doing Windows development, first in VB, then in c#. I think my most common mistake is to implement things that are already there. For instance I once did a batch application that converted a huge amount of insurances from several systems into one system on a daily basis. Performance was really an issue, but I did it in VB. DTS would have been much quicker. Similarly, in my current c# system I needed to save some objects as XML files and then restore them. I wrote a lot of code for this, not knowing about XML serialization.

A variant of this "error" is when everyone see their own tool as the solution to the problem. The SAS developer want to do everything in SAS. The Notes developer want to do everything in Notes. The VB programmer want to do everything in VB.

A common error I have seen other programmers (or rather designers) do is overdesigning everything into layers, so that there are data layers, business entity layers, business process layers that all just move data.

Another design mistake I see very frequently is when people start with the physical design of the system. They say "we have a layered system"; and start talking about different tiers (data tier, middle tier, presentation tier). Personally, I like to start with a logical design, and address the physical design later (but still in the design phase - if the physical design is omitted altogther one will not have a clue about performance and scalability).

Another common error is to "smart-code" because it will run faster, when, in most cases, the performance of individual pieces of code really not is an issue.

Also, like Darwen I dislike pointless comments. Copy-pasted comments. Misspelled comments. Omitted comments. Commented code.

[ovidiucucu]

Of course all the standard ones stand : like 1000 line methods with huge blocks of commented out code in them.
More like that, the methods of 1000+ effective lines of code...
Once, one of my colleagues added a single line to such a method and got a compiler limit error. He was so happy, that unplugged his computer and went home (or, I'm not sure... to drink something strong). :D ;)

[RoboTact]

I can only say, I'm glad I don't use any of your software - I'd hate to be a user, be working along quite nicely and have a message box with "error 0x8020304" come up and my last 6 hours work be wiped out for no reason.Would you be happier then, if with 6 hours work wiped you'll receive "internal error in function CreateThatSystemObject: wrong argument" message? Devastating bugs is completely different matter from spellable error messages.
I'm amazed that someone actually thinks that reporting an 0x error is acceptable.Who is to read such errors and make some actions or decisions based on them? If it's manageble for user - such as "file not found" or "can't write to disk" - then yes, but if it's bug in software, why user need to know details? He only need a way to provide that error information to developer.

I bet you use STL and have huge memory leaks in your code you can't be bothered fixing too. And even if you wanted to fix the memory leaks, you wouldn't be able to - using STL and all. How much would you bet? ;)

[ovidiucucu]

No, really error doesn't need to be displayed in spellable form, there should only be the way to give developer enough hint about where the problem is. In many cases source line number/IP is enough
Unfortunatelly, in many cases the source file/line number no use even for the developer.
My example (http://www.codeguru.com/forum/showpost.php?p=1276713) shows that, that message not use at anything even, let's say, we include __FILE__ and __LINE__ in message string.
Moreover 0x80004005 (E_FAIL) means "Unspecified error" and say nothing either to user or developer.

My given example was picked from real code in a "reports" module. That was fixed and now the user can see from time to time, let's say, "Default printer not connected" instead of "0x80004005".

You can observe not all errors are catastrophic and in more cases if they are correct handled and meaningful reported, the user can easily see that forgot something and not hurry to report it as a bug.

[ovidiucucu]

Would you be happier then, if with 6 hours work wiped you'll receive "internal error in function CreateThatSystemObject: wrong argument" message?
Yes. At least we'll know who to poison, shoot, then hang. :D Well, I'm kidding. ;)

[ovidiucucu]

Who is to read such errors and make some actions or decisions based on them? If it's manageble for user - such as "file not found" or "can't write to disk" - then yes, but if it's bug in software, why user need to know details? He only need a way to provide that error information to developer.
It seems you have chosen wrong examples. The real bugs are the given messages. :)
Instead, "File 'C:\MyCoolApp\imports\importx.csv' not found" and "Can't write to C: drive. Disk full" can be extremely useful for the user also.
For the real situations that concern only the devloper and not the user, the DEBUG mode (with using trace and assertion functions) was invented.
Unfortunately, many "programmers" use asserts in many wrong places like in this example (http://www.codeguru.com/forum/showpost.php?p=1096539).

[RoboTact]

Well, it weren't examples, just names of error types...
What's completely wrong with that ASSERT? it's just that error isn't reported, but if it's really asserted at that point that function succeeds, it's OK. Question is if you can assert such thing.

[ovidiucucu]

I gave a brief explanation HERE (http://www.codeguru.com/forum/showpost.php?p=1208494).
Need more, eventually snippet code, examples, demo application? ;)

[SuperKoko]

You will never belive but some years ago I have found something like this in project which was on last test before release :eek:

p=malloc(bytes);
if(p==NULL)
{
ShowMessage("Can`t allocate enough memory");
exit(EXIT_FAILURE);
}


So this software (it was written on C language as you see) if can`t find some more bytes for new data object closed itself, and even not ask user to save his work.
That is not because it contains only valid C code that it was written in C (of course, you have seen the true project, so, you know).

I just quoted your text to introduce the fact I found some "apparently C codes" which revealed, after a further examination, to be C++ (pass by references used somewhere for example).

[Zorro3D]

I have often seen this which can lead to problems:

if(result = DoSomeStuff( data))
{
...
}

And this is common for beginners and is particularly hard to find in order to fix:

if(this = that)
{
do_stuff();
}

Being as 'this' will become 'that' and will always equal that. So this conditional will always be true.

Maybe not a 'beginners' error:

if(object->next->value == value)
{
do_stuff();
}

When there is no reason to assume that the 'next' object actually exists...

:cool:

[NMTop40]

I have often seen this which can lead to problems:

if(result = DoSomeStuff( data))
{
...
}

You can make it clearer by saying
if ( ( result = DoSomeStuff( data ) ) != 0 )
{
...
}

I have been known not to bother to put the !=0 comparison it. It's usually obvious that you are intending to assign result and continue into the block if it's non-zero.

And this is common for beginners and is particularly hard to find in order to fix:

if(this = that)
{
do_stuff();
}

Being as 'this' will become 'that' and will always equal that. So this conditional will always be true.

won't compile. this is not an l-value.

Maybe not a 'beginners' error:

if(object->next->value == value)
{
do_stuff();
}

When there is no reason to assume that the 'next' object actually exists...

:cool:
and if this is done in C++ it's a beginners error to be using their own list and not std::list and algorithms. In fact it's a beginners error to use linear search in situations where either std::map or a sorted vector would be a better idea.

[cilu]

I go crazy when I see something like this:

#define AFUNC1(a, b) AFunc(a, b)
#define AFUNC2(a) AFunc(a, NULL)

void AFunc(int a, const char* b)
{
// don something
}

...
AFUNC2(10);

And I've seen code like that.

And I wish I had a gun when I see things like this:

#include <iostream>

using namespace std;

#define BEGIN \
if(index<0 || index>=5) \
return; \
goo temp = _goo[index]; \
temp.Init(); \

#define END \
temp.Close();

class goo
{
public:
goo(){}
goo(const goo& cpy) {}
const goo& operator=(const goo& rval) { return *this;}
void Init() { cout << "goo::init" << endl;}
void Action() {cout << "goo::action" << endl;}
void Reaction() {cout << "goo::reaction" << endl;}
void Close() {cout << "goo::close" << endl;}
};

class foo
{
goo *_goo;
public:
foo();
~foo();
void do_something(int index);
void do_something_else(int index);
};

foo::foo()
{
_goo = new goo[5];
}

foo::~foo()
{
delete [] _goo;
}

void foo::do_something(int index)
{
BEGIN
temp.Action();
END
}

void foo::do_something_else(int index)
{
BEGIN
temp.Reaction();
END
}

int main()
{
foo f;
f.do_something(0);
f.do_something_else(4);

return 0;
}

[ovidiucucu]

What do you think? This one...
for(int i = 0; i < nCount; i++)
{
try
{
// something...
}
catch(...)
{
i --; // try again
}
}
... could be funny, stupid, or it's really a bad joke?