Bad previous code. To fix or not to fix?
- by Viniyo Shouta
As a freelancer programmer I am often asked to edit part of an application source code in order to add functionalities, fix bugs etc. While I'm on my adventure journey to study the source to do what I'm asked correctly I run into code like:
World::User* GetWorld()
{
map<DWORD,World*>::iterator it = mapWld.find( m_userWorldId )
if( it != mapWld.end() )
return &it->second;
return NULL;
}
if( pUser->GetWorld()->GetId() == 250 )
If I investigate further I end up finding that the DWORD class member of User, userWorldId can be a value non-found in the map mapWld, which will lead to a casuality as also known as crash! The obviously valid way to do it is:
World* pWorld = pUser->GetWorld(); if( pWorld && pWorld->GetId() == 250 )//...
Sometimes when it's something just 'small' I end up sort of 'fixing' it. But sometimes when I'm on a 500 thousand line source code and this kind of code is everywhere there is no much can do. The question is if it's politically correct to fix some of these things. Think of it;
You are not paid to fix it.
Perhaps you think it's right, but it was necessarily done that way for some reason and you should not be messing with it.
You do not have authorization, you do not own the source and none of the copyrights belong to you.
You have authorization to edit issues accordingly to the owners but you're in a hurry, you have many other projects to do, it's the end of the month, you must pay the bills.
Sincerely, I think of it as seeing an animal die from a disease in front of you, you have the cure in your hands but you do nothing. What is the best to do in this scenario?
