|
CIDev wrote: there are likely other bad practices I found the same piece of code something like five times (EXACTLY the same). And then other pieces of code five times too. I found slightly different code even more often (don't make a function with input parameters, copy paste the code and change a word or two).
I found functions that were called ChangeX(ByVal x As Integer) that also changes Y and Z, ChangeY(ByVal y As Integer) that also changes X and Z etc. Worst part is, this guy coded them and still called ChangeX, ChangeY and ChangeZ one after the other.
Worst has still to come... This guy is technical director and pretends to be quality insurer (he is, but doesn't really insure anything).
He has told me I should not code MessageBox.Show("Hello"), I should've done:
Dim prompt As String = "Hello"
MessageBox.Show(prompt)
because... I don't know... He really doesn't understand I guess! If he would've coded it he wouldn't even have typed 'As String' because that's only inconvenient.
I've even seen him use switch statements for booleans! Somehow this guy doesn't like If statements...
And this guy is 'by the book' (really, if he sees something he doesn't like he always has a book that defends his position, it's freaky!)...
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Yeah, that does sound pretty scary. It is expecially bad that he is a technical director.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
At our place we have a few of the "senior" apprentices staying with us in the dev team for a year. When I say "senior", they're about 19 in human years, but look about 8. I got to mentor one for a few weeks and threw him in at the deep end by giving him a MOSS WebPart to develop. For the most part, he's done an OK job and seems to be getting the hang of things, but still comes out with stuff like this:
int status = (int)(Status)Enum.Parse(typeof(Status), _statusBy.ToString());
this.dataSource.SelectParameters.Add("status", status.ToString());
Given that Status is an enum with integer values, this would actually suffice:
this.dataSource.SelectParameters.Add("status", ((int)_statusBy).ToString());
Bless.
|
|
|
|
|
At least the kid theoretically understood what he wanted to accomplish. I'm guessing that you showing him the "correct" way will result in them learning as opposed to doing it over and over again. (Silver lining)
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
A 19 year old producing a working but slightly inelegant solution isn't really a horror. I hope he takes your tips on board and learns the little tricks like this.
|
|
|
|
|
That is far better than some of the garbage I have seen well-aged developers who have been with the company for years do. From them, I'd expect something more like this:
if (Page.IsPostBack == false)
{
int status = -1;
if (_statusBy == Status.FirstStatus)
{
status = 0;
}
if (_statusBy == Status.SecondStatus)
{
status = 1;
}
if (_statusBy == Status.ThirdStatus)
{
status = 2;
}
litStatus.Text = status.ToString();
}
string strCmdMySqlCmd = "SELECT * FROM myTable WHERE mtStatus = " + litStatus.Text + " AND mtRecordNumber = " + Request.QueryString["PleaseInjectSomeSql"];
myDataSource.SelectCommand = strCmdMySqlCmd;
Count yourself lucky to have such a bright apprentice.
|
|
|
|
|
AspDotNetDev wrote: That is far better than some of the garbage I have seen well-aged developers who have been with the company for years do
Don't get me started, I'll be here all night and it's a friday.
AspDotNetDev wrote: Count yourself lucky to have such a bright apprentice.
I do, don't get me wrong - I was just surprised that despite his ability to understand and get on with things fairly autonomously he still does some slightly daft stuff. For a measure of how bright he is, he already hates MOSS development, and that after only a couple of weeks. Clever lad.
|
|
|
|
|
jim lahey wrote: despite his ability to understand and get on with things fairly autonomously he still does some slightly daft stuff Don't we all do that from time to time?
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Lol we have this code base written by our german counterparts from like 10 years, and i am sure most of them are atleast in their 40+ age having atleast 10+ years of experience. Lol their code starts with main and ends every god damn thing in it only. There are many surprising methods with 2000 lines of code in it.
|
|
|
|
|
We've got them too. I have to rely on an in house component that dynamically changes the datasource in a particular GIS file type. You have to give it the path to the file you want to change and a new filename for the changed version. If you pass it two nulls or two empty strings or two strings that have never been anywhere near a valid path or filename, it runs through without a peep. If you pass it a null or empty path and a valid new file name it creates an empty file with the name you gave it. If you feed it with correct parameters it also runs through without making any noise, so every single piece of software that uses this component has to check the paths are valid and that the created file at least isn't empty.
I guess it's taking the single responsibility principle a little too far
|
|
|
|
|
Apparently, my predecessor didn't trust in the C# typing system and... there's more that's wrong with this:
[XmlIgnore]
public DateTime? SomeProperty
{
get;set;
}
public String SomePropertyXml
{
get
{
return SomeProperty.HasValue ? XmlConvert.ToString(SomeProperty.Value) : null;
}
set
{
if (String.IsNullOrEmpty(value as String))
SomeProperty = null;
else
SomeProperty = XmlConvert.ToTimeSpan(value);
}
}
Oh, and I have removed identifiable identifiers, but I haven't removed any comments
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
Apparently this was written before the Big Bang when the concept of time didn't exist, thus the need for a nullable DateTime property
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
Oh, that's not even strange in my codebase. We have many nullable DateTime, as optional parameters/informations.
For instance, we have a kind of 'last access date' for files, which is stored as a Nullable<datetime>.
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
You don't access it on creation? Show me, how do I bake a cake without touching it?
Bastard Programmer from Hell
if you can't read my code, try converting it here[^]
|
|
|
|
|
Ok, I'll be more precise then.
I'm working on a media player application, in which you get a list of media files from a webservice.
The DateTime of last playback is null until you have actually played the file.
Does that make sense now?
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
Makes sense to me ... if you don't know what date something happened on, or if it didn't happen yet, both of which are quite common.
|
|
|
|
|
Clearly your predecessor did not understand either the type system nor TimeSpans.
Just because the code works, it doesn't mean that it is good code.
modified 20-Sep-12 13:11pm.
|
|
|
|
|
Likely both.
I think computer viruses should count as life. I think it says something about human nature that the only form of life we have created so far is purely destructive. We've created life in our own image.
Stephen Hawking
|
|
|
|
|
I've seen stranger DateTime usage in the Web Service I'm working on. We use a nullable type too, but in this case it was a WTF because Microsoft handed us a WTF with WCF. Stranger is our code that handles a DataSet which has a DateTime column as part of an incoming web request. Whenever we get new developers on board, we watch them when they come across this piece of code. Some of them slump down in their desks and go into a trance, while some quickly walk away and wash their hands with plenty of SOAP.
SG
Aham Brahmasmi!
|
|
|
|
|
Here's a 'nice' pattern I saw in some code I've inherited...
catch (Exception ex)
{
if (ex is ExceptionSpecialType)
throw new SomeException()
{
CustomData = (ex as ExceptionSpecialType).SomeThing.ToXmlString()
};
else
{
throw new SomeException()
{
CustomData = new CustomError(ex).ToXmlString()
};
}
}
Oh, and yes, my predecessor loved using 'if (X is T) (X as T).M();'.
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
And I thought my custom unrecoverable Exception handling wrapper sucked:
Helper.logUnrecoverable(this.GetType().ToString(), "doSync - createHeader or createLine fail. Action rollbacked", e.Message, true);
Where true is whether it should log the StackTrace or not. But this is a whole different -ness level
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
Obviously, your predecessor did not have the slightest understanding of proper exception handling. Even reading one article on Code Project about exception would have helped him greatly
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
I've worked with a codebase where each and every method has a try catch block.
The catch logged the error and that's that. As a result, every method you called would return normally like everything went fine.
Something like an Exception or unexpected results when updating a database record could very well be the result of a ReferenceNull- or InvalidCastException somewhere in some method.
Good luck trying to find it
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
"Look boss, never any error!"
In Visual Studio, you can set a kind of breakpoints on exceptions just when they're thrown, I guess it could help with that kind of monstrous codebase.
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
Julien Villers wrote: it could help with that kind of monstrous codebase We would sit and cry in the corner waiting for help... Help never came
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|