Spot the problem in this code... I've just summarily rejected a piece of code from a developer, and they're being really really pissy about it, they're in fact being so pissy they've been moaning about me in the kitchenette at the side of the office, and the people they've been pissing and moaning at about me have been coming to me laughing that the guy can't work out that he's in the wrong and his code is shite.
Now, I'm not able to post the full code, it's a commercial product, however, I will give you a quick over view... Here's a sort of example:
#include <iostream>
#include <sstream>
#include "CString.h"
char* IntToString (int value)
{
std::ostringstream s;
s << value;
return s.str().c_str();
}
int main (int c, char** v)
{
CString str (IntToString(c));
std::cout << "There are: " << str << " parameters" << std::endl;
}
Now, this is pretty much how part of his program looks, that "IntToString" function is taken straight from his code, and it's horrible, but he really can't see why...
So, can you see why folks?...
Right, well, first things first, and this is the elephant in the room... Why the fuck has he reimplemented String?... He's written his own "CString" function to wrap character pointers as strings of characters, and he's done a bad job of that because he's done this:
char* buffer = new char[256];
delete buffer;
Yeah, see the mistake?... He doesn't, I've pointed out "delete[]" to him several times elsewhere.
But, yes, despite using the standard library for "string stream", he's not used "std::string"...
So, changing the code above:
#include <iostream>
#include <sstream>
#include <string>
const std::string IntToString (const int& value)
{
std::ostringstream s;
s << value;
return s.str();
}
int main (int c, char** v)
{
std::string str = IntToString(c);
std::cout << "There are: " << str << " parameters" << std::endl;
}
So, we've used "std::string", we've fixed the return type to be a constant, and also made the value input to the function a constant reference, so we don't copy the value into a new integer before passing to the function - I know a 4 byte integer is trivial - bit imagine copying a 2MB class, or a 35mb image before passing it!
However, I'm still not happy with this code... In this project, in all my C++ projects the boost libraries are always made available, they're built ont he platform for the project, and go along with it as a "contributing" library of helpers, to stop people reinventing the wheel. On the office wall, printed on the notice board in the very kitchenette the moron has been moping around in there is a print out of several template functions we lilke to use.
These template functions are also included in a "Framework.hpp" file, which I tell all contributors to use, inside there is a function:
#include <string>
#include <boost/lexical_cast.hpp>
namespace Framework
{
class Helpers
{
private:
Helpers() {}
public:
template <typename T>
static const std::string ToString<T>(const T& p_Value)
{
std::string l_result("");
try
{
l_result = boost::lexical_cast<std::string>(p_Value);
}
catch(const boost::bad_lexical_cast&)
{
// Report
}
return l_result;
}
template <typename T>
static const T FromString<T>(const std::string& p_Text)
{
T l_temp;
try
{
l_temp = boost::lexical_cast<T>(p_Text);
}
catch (const boost::bad_lexical_cast&)
{
// Report
}
return l_temp;
}
};
}
These functions let you convert any type to and from text easily, and you don't reinvent the wheel...
Someone did point these out, "Why not just use helpers in the framework?"...
Which would tell me to do "Framework::Helpers::" and see what the code completion came up with, at the very least, or just to go look at the Framework.hpp file, this guys response was apparently:
"I can't, the constructor is private, and when I make it public I can't see the to and from string functions in the instance of the class I create"
Just, the... ah... This is a "graduate" programmer, who considers (at least according to his CV) himself an expert user of C++ for over 4 years, expert in C++11.
I swear to god, the guy is testing my patience to the max, not least because he just moans all the time, and most of the time, it's just his being shite.
I mean, I've moaned before, moaned whilst coding systems, whilst coding parts of things, sometimes warranted, sometimes not, but I don't think I've ever put my foot as far in my mouth as this guy has ever, ever! And I once accidentally text'd a manager pondering why said manager was such a wanker!
Hey ho, I wish the office was not open plan, I'd like to take the guy aside and explain to him these problems, thede deficiencies in his knowledge, but there's no private space, I may send him a mail asking him to hang back to talk when others have floated out, most of the other developers disappear after 4, and all are usually gone by 6; it's only myself doing the later hours in general.
No comments:
Post a Comment