Frequently encountered errors

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

Code:

---

  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.

Quote:

---

Originally Posted by ovidiucucu

Returning to C/C++
This one makes me often feeling sorry I have not a machine gun:

Code:

---

  HRESULT hr = SomeFunction();
  _ASSERTE(SUCCEEDED(hr));

---

---

what's the code you suggest, then?
I'm a beginner...

Quote:

---

Originally Posted by xufeisjtu

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.
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.

Quote:

---

Originally Posted by darwen

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

Code:

---

  // TODO: Add your specialized code here and/or call the base class

---

or

Code:

---

  return TRUE; // return TRUE unless you set the focus to a control
                // EXCEPTION: OCX Property Pages should return FALSE

---

:D

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

Code:

---

class CMFxTVp
{
}

---

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

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

Code:

---

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.

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:

Code:

---

// ...
  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.

;)

About using of HRESULT again:

Code:

---

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).
  }

---

Quote:

---

Originally Posted by ovidiucucu

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

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:

Code:

---

void doSomething(){
  if(thisIsFine() && thatIsFine())
  {
    // actually do that something
  }
  // no error reporting
}

---

or like this:

Code:

---

void ShowThatLittleDotInTheCorner()
{
  if(somethingWentWrong())
    TerminateWholeApplicationWithoutAnyReport();
}

---

Quote:

---

Originally Posted by Skoons

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).

Quote:

---

Originally Posted by RoboTact

[/code]or like this:

Code:

---

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 ;)

Quote:

---

Originally Posted by aio

- 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.

Quote:

---

Originally Posted by Andy Tacker

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!

Quote:

---

Originally Posted by Andy Tacker

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

---

There are many guys who think that:

Code:

---

something && something_else;

---

Is faster than

Code:

---

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++;"

Quote:

---

Originally Posted by Norfy

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:

Code:

---

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

From RoboTact :

Quote:

---

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.

Quote:

---

Originally Posted by darwen

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.

Quote:

---

Originally Posted by darwen

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 ;)