The Weird and The Wonderful

hahaha lol... clever idea Big Grin | :-D

============================================
The grass is always greener on the other side of the fence

So my boss is like a rabbit. He just loves his greens.

Unclear naming should stay unclear by commenting it (helper was never used, but that's beside the point).

VB

' A helper variable.
Dim helper As Boolean

Of course we could make clear comments in favor of properly naming our variables.

VB

' s is a SalesOrder
s.DoSomething()

p.DoSomethingElse ' PurchaseOrder

The obvious also needs extra explanation.

VB

i = nX + nY ' Add nX and nY

In case we ever want to compile our code into a beginners how-to-program book.

VB

' Assign the value to the variable
i = 10

We just can't have enough comments!

VB

' Refresh the screen
RefreshScreen

Keep an active log inside your code, you never know when you need it (SVN alone isn't good enough)... (date, initials, ticket number).
The fun thing about this is that when a fix is larger than a few code files it does not have to be commented.
If it is only a few code files chances are this piece of commentary is copied to each of them, sometimes in multiple places in the same file.

VB

' 2012-09-22 N Ticket # 666.
' The client wants this fix because... Bla, bla, bla...
CallSomeMethod

Some of this guys methods are commented line by line with these kinds of comments... WTF | :WTF:

I tried to talk to him about it, but he's convinced code is unreadable and can never be understood easily by just reading it. Comments are absolutely necessary if you, or another programmer, ever want to quickly make changes to the code. This is the same guy who thinks making a seperate class for almost every method (and randomly make it non-shared/non-static or shared/static) makes code more clear and readable (and single responsibility principle is a bonus!) and who just doesn't use access modifiers out of laziness or indifference...
Of course he is my boss and has eight years of experience where I have only two (how often I had to hear that) so when I disagree with him I'm rude and inexperienced.
I guess someday I'll laugh about it Unsure | :~

It's an OO world.

C#

public class Naerling : Lazy<Person>{
    public void DoWork(){ throw new NotImplementedException(); }
}

Naerling wrote:
Keep an active log inside your code

Ugh. I hate that. At my last job, they were all about that. Half a million lines of SQL, and none of it in version control (it was all nearly lost once, except I and another person had a personal backup).

Naerling wrote:
I guess someday I'll laugh about it

When the company collapses and you find a new job with reasonable code practices.

Thou mewling ill-breeding pignut!

AspDotNetDev wrote:
When the company collapses and you find a new job with reasonable code practices.

Nah, they've been good to me and though I disagree with their code practices (and I disagree on almost everything with the guy who writes these comments) I don't want them to collapse... The funny thing is that the guy who writes these comments taught me to code! Luckily I surpassed my 'master' in every way (after about a year) Smile | :)

Although he's still the better SQL Server guy.

It's an OO world.

C#

public class Naerling : Lazy<Person>{
    public void DoWork(){ throw new NotImplementedException(); }
}

Lack of decent version control is always a bad sign. Frown | :(

Just because the code works, it doesn't mean that it is good code.

(Really) Bad smell, think we should refactor!

I like to comment my code, but only where really necessary. I also use meaningful variable names and method names so that the code is largely self-commenting. It is rare to see someone who wants both pointless comments and meaningless names, fortunately. Wink | ;)

If those things are wrong there are likely other bad practices and I would think about looking for a better place to work.

Just because the code works, it doesn't mean that it is good code.

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.

C#

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. Sigh | :sigh:

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:

C#

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:

C#

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:

C#

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.

Thou mewling ill-breeding pignut!

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? Smile | :)

It's an OO world.

C#

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. Sleepy | :zzz:

My cUr10U5 w0rlD

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 Smile | :)

Apparently, my predecessor didn't trust in the C# typing system and... there's more that's wrong with this:

C#

[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 Frown | :-(

'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 WTF | :WTF:

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? Poke tongue | ;-P

Bastard Programmer from Hell Suspicious | :suss:

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

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