Coding Horror

Mr. CooL ICE · 21-03-2017 1:27pm

Not exactly a horror, but it still made me go "huh?":

    switch (isValid)
    {
        case true:
            // do something
            break;
        case false:
            // do something else
            break;
     }

BrokenArrows · 11-04-2017 2:44pm

Just been handed a new project with what initially seemed like a simple change request but ive had to spend the last two days trying to figure out how the whole project works and its insanely over engineered, I mean it could have been very simple but this guy seems to have written this code like he had to justify the amount of time he quoted for the work.

And there is no comments, not just few comments or bad comments. There is not one single comment line anywhere in the code. Ahhhhhhh!!

BrokenArrows · 24-04-2017 3:56pm

Ahh this project is driving me crazy. The guy who wrote it does everything in such an unintuitive way.

Take this switch logic for example:

private bool DoStuff(MyEnum value)
{
    [B]UploadFilePath = "MyFiles1\PendingUploads";[/B]

    switch(value)
    {
        case MyEnum.Option2:
            UploadFilePath = "MyFiles2\PendingUploads\";
            break;

        case MyEnum.Option3:
            UploadFilePath = "MyFiles3\PendingUploads\";
            break;
    }
}

So the line in bold is only valid when its passed with MyEnum.Option1

WTF, Just add it into the switch. I know its minor but when the code is full of these little quirks it wrecks my head.

Maximus Alexander · 24-04-2017 4:52pm

BrokenArrows wrote: »

So the line in bold is only valid when its passed with MyEnum.Option1

It's also valid if null is passed. Though this may or may not be desirable.

Tow · 24-04-2017 5:11pm

BrokenArrows wrote: »

WTF, Just add it into the switch. I know its minor but when the code is full of these little quirks it wrecks my head.

Old school developer style...

Wombatman · 25-04-2017 9:19am

UploadFilePath = "MyFiles1\PendingUploads";

Bug found. Backslash required at end of file path above.

BrokenArrows · 25-04-2017 12:07pm

Maximus Alexander wrote: »

It's also valid if null is passed. Though this may or may not be desirable.

Enums cant be null. (Well at least they cant in C#)

Wombatman wrote: »

Bug found. Backslash required at end of file path above.

Haha. Actually there is a bigger problem that i didn't escape the first \ and this wouldnt compile!!

:D

Maximus Alexander · 25-04-2017 12:37pm

BrokenArrows wrote: »

Enums cant be null. (Well at least they cant in C#)

D'oh! My Java is showing. Though Java would throw a different exception because you can't have null in a switch.

Anesthetize · 25-04-2017 8:38pm

Wombatman wrote: »

Bug found. Backslash required at end of file path above.

Hard-coded file separators are a coding horror in themselves

File paths should be compatible with both Windows and Linux.

djpbarry · 03-05-2017 4:30pm

Maximus Alexander wrote: »

It's also valid if null is passed. Though this may or may not be desirable.

ALWAYS check for null!!!

Even if the above were true, default behaviour should be specified with the default keyword in the switch.

SimonTemplar · 06-06-2017 9:23pm

What the hell. WHAT THE HELL.

Has this guy ever heard of returning an object?! Not to mention those multiple unnecessary db calls when he is only using one field each time! Ugh!

        private void processAddress()
        {           

            ....

            string addressline1 = getAddressData("addressline1", customerID);
            string addressline2 = getAddressData("addressline2", customerID);
            string addressline3 = getAddressData("addressline3", customerID);
            string addresscity = getAddressData("addresscity", customerID);
            string addresscountry = getAddressData("addresscountry", customerID);
            string addresspostalcode = getAddressData("addresspostalcode", customerID);

            ....

        }

        private string getAddressData(string dataToGet, string customerID)
        {
            string query = "SELECT addressline1, addressline2, addressline3, addresscity, addresscountry, addresspostalcode FROM addresses WHERE customerID = " + customerID;
            string dataToReturn = "";

            if (this.OpenConnection() == true)
            {                
                MySqlCommand cmd = new MySqlCommand(query, connection);                
                MySqlDataReader dataReader = cmd.ExecuteReader();
                
                while (dataReader.Read())
                {
                    switch (dataToGet)
                    {
                        case "addressline1":
                            dataToReturn = dataReader["addressline1"].ToString();
                            break;
                        case "addressline2":
                            dataToReturn = dataReader["addressline2"].ToString();
                            break;
                        case "addressline3":
                            dataToReturn = dataReader["addressline3"].ToString();
                            break;
                        case "addresscity":
                            dataToReturn = dataReader["addresscity"].ToString();
                            break;
                        case "addresscountry":
                            dataToReturn = dataReader["addresscountry"].ToString();
                            break;
                        case "addresspostalcode":
                            dataToReturn = dataReader["addresspostalcode"].ToString();
                            break;
                    }
                }                
                dataReader.Close();
                this.CloseConnection();
            }
            return dataToReturn;
        }

Raging_Ninja · 21-06-2017 9:13pm

found this at work (variable names obfuscated of course):

bool  wtf =  omg != true ? (ffs != true ? (fml != true ? true : false) : false) : false;

Buford T Justice · 21-06-2017 11:49pm

Raging_Ninja wrote: »

found this at work (variable names obfuscated of course):

They could have at least done

bool wtf = !omg ? (!ffs ? (!fml ? true : false) : false) : false;

pillphil · 22-06-2017 1:20am

Buford T Justice wrote: »
They could have at least done
bool wtf = omg ? false : (ffs ? false : (fml ? false : true));

What are they trying to say here?

!omg && !ffs && !fml?

Raging_Ninja · 22-06-2017 10:36pm

pillphil wrote: »

What are they trying to say here?

!omg && !ffs && !fml?

Well, thats what I turned it into anyways.

BrokenArrows · 23-06-2017 10:09am

Raging_Ninja wrote: »
found this at work (variable names obfuscated of course):
bool  wtf =  omg != true ? (ffs != true ? (fml != true ? true : false) : false) : false;

Jesus. Haha.

BrokenArrows · 10-07-2017 11:22am

Started a new project last week. Summary of the project would lead you to believe its a very straight forward thing which i can knockout in a few days.

I just needed to interface to another companies API and they have a published WSDL. What could be easier!!!

Where to begin:

1. The API has 1 method defined

string responseXML DoTransaction(string xmlRequest)

Thats enough to boggle the mind.

The obvious next problem is getting them to send me detailed information of the xmlRequest and response! Many many emails later with varing spelling and casing in the xml they send me a valid working example.

Ok Progress.

2. The responses. Logic would make you think that the response format should be consistent. But no. Depending on the request made the spelling and casing of the xml changes so im going to have to write a specific deserializator for each request type to take into account their xml response with different spelling and casing.

ahhh!! idiots.

3. Success and Failure responses. The response contains a response code, response message. You would think that the response message would be error when it fails, but no it says success even when the error code indicates failure.

4. Jesus, just found another one. Depending on the type of error encountered the casing of the xml in the response changes. FFS. Am i going to have to write some kind of case insensitive deserializer for this ****.

5. One more, their parser throws a parser error if you include an xml declaration at the start of your xml. F

Wombatman · 01-08-2017 1:26pm

oscarBravo · 01-08-2017 1:44pm

Wow. Looks like a Java programmer with Python envy.

BrokenArrows · 01-08-2017 2:01pm

Wombatman wrote: »

WTF is that. That must have been someone leaving the company pissed off.

Raging_Ninja · 01-08-2017 2:59pm

BrokenArrows wrote: »

WTF is that. That must have been someone leaving the company pissed off.

Waste of time if so, most IDEs have an auto indent feature.

[Rasta] · 01-08-2017 6:57pm

My manager for the third/fourth time in a few months sent me some TSQL code and asked whether it was close to following our coding style.
As I had previously noted to the manager that the stuff they checked in was unreadable.

This is basically what I was sent:

select
	john as Doe,
	
	a.jim_meab [hmm]
,dat.another_field [an_fi]
,dat. RANDOM_STUFF AS [an_fi1]
,dat.whatisgoingon [an_fi2]
		,OHMYGOD&#160;
		as ineedhelp,
		dat.makeitstop
	from mytable a
inner JOIN dbo.cheese ON cheese.somevalue = a.anothervalue
	LEFT OUTER JOIN
	
	(select morestuff x	
		, table.copypasta CopyPasta, goodluckreading.THIS
		from stuff left join table on stuff.x =
		table.y			
			LEFT JOIN goodluckreading ON table.y = myotherval
		
				) dat on x='1' 
and copyPasta=otherthing
	WHERE
	copypasta IS not null

After, I didn't reply, the manager came over and asked about it, I said NO and I turned my head back at my screen.
What the hell?

BrokenArrows · 09-08-2017 10:33am

[Rasta] wrote: »
My manager for the third/fourth time in a few months sent me some TSQL code and asked whether it was close to following our coding style.
As I had previously noted to the manager that the stuff they checked in was unreadable.

This is basically what I was sent:
select
	john as Doe,
	
	a.jim_meab [hmm]
,dat.another_field [an_fi]
,dat. RANDOM_STUFF AS [an_fi1]
,dat.whatisgoingon [an_fi2]
		,OHMYGOD&#160;
		as ineedhelp,
		dat.makeitstop
	from mytable a
inner JOIN dbo.cheese ON cheese.somevalue = a.anothervalue
	LEFT OUTER JOIN
	
	(select morestuff x	
		, table.copypasta CopyPasta, goodluckreading.THIS
		from stuff left join table on stuff.x =
		table.y			
			LEFT JOIN goodluckreading ON table.y = myotherval
		
				) dat on x='1' 
and copyPasta=otherthing
	WHERE
	copypasta IS not null
After, I didn't reply, the manager came over and asked about it, I said NO and I turned my head back at my screen.
What the hell?

Does your coding style match any auto format tools? Maybe suggest they just run it though an auto format tool.

BrokenArrows · 11-08-2017 11:10am

Just wasted some time trying to workout why i was getting versioning issues. Some muppet put the assembly version into the visual studio csproj file for an unknown reason. Still not sure why this is causing problems because all the version numbers match whats in the assemblyinfo.cs