The Weird and The Wonderful

The Weird and The Wonderful forum is a place to post Coding Horrors, Worst Practices, and the occasional flash of brilliance.

We all come across code that simply boggles the mind. Lazy kludges, embarrassing mistakes, horrid workarounds and developers just not quite getting it. And then somedays we come across - or write - the truly sublime.

Post your Best, your worst, and your most interesting. But please - no programming questions . This forum is purely for amusement and discussions on code snippets. All actual programming questions will be removed.

Re: String.Format???

Jörgen Andersson9-Jul-10 13:00

Jörgen Andersson

9-Jul-10 13:00

My wife is going to kill me, I'm supposed to put up new wallpaper in the livingroom tomorrow. Not sit in front of the computer again.

"When did ignorance become a point of view" - Dilbert

Re: String.Format???

PIEBALDconsult9-Jul-10 13:05

PIEBALDconsult

9-Jul-10 13:05

It'll keep.

The wallpaper, that is. Big Grin | :-D

Re: String.Format???

PIEBALDconsult9-Jul-10 15:05

PIEBALDconsult

9-Jul-10 15:05

I finally remembered to try SELECT * FROM Account WHERE EXISTS ( SELECT ID FROM @Param0 IDs WHERE IDs.ID=Account.ID ), it works.

It appears that one of the differences between IN/EXISTS and JOIN is that the JOIN sorts (or maybe the others do).

Re: String.Format???

Jörgen Andersson9-Jul-10 10:32

Jörgen Andersson

9-Jul-10 10:32

PIEBALDconsult wrote:
Personally I wouldn't. I wouldn't use IN at all, I'd find a way to have a table on which to JOIN instead.

A JOIN is a poor substitute for a NOT IN or NOT EXISTS. And they don't handle nulls the same way.

PIEBALDconsult wrote:
The statement you present is a symptom of a poorly implemented system.

Agreed, but sometimes that's what you have. Sigh | :sigh:

"When did ignorance become a point of view" - Dilbert

Re: String.Format???

PIEBALDconsult9-Jul-10 10:45

PIEBALDconsult

9-Jul-10 10:45

But your example uses IN, not NOT IN. Big Grin | :-D

Re: String.Format???

Jeremy Hutchinson9-Jul-10 8:54

Jeremy Hutchinson

9-Jul-10 8:54

I would imagine this would work:

string query = "select * from table where ID in (@value1, @value2, @value3)";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value1", 123);
cmd.Parameters.Add("@value2", 124);
cmd.Parameters.Add("@value3", 125);

If not, this surely would:

string query = "select * from table where (ID = @value1 or ID = @value2 or ID = @value3)";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value1", 123);
cmd.Parameters.Add("@value2", 124);
cmd.Parameters.Add("@value3", 125);

Re: String.Format???

Jörgen Andersson9-Jul-10 11:11

Jörgen Andersson

9-Jul-10 11:11

It does as long as your list isn't dynamic, but assume your list has a varying number of values...

"When did ignorance become a point of view" - Dilbert

Re: String.Format???

CDP180211-Jul-10 20:43

CDP1802

11-Jul-10 20:43

That is true, things get a bit problematic when you have many or a variable amount of parameters.

First, I would simply try to avoid the situation if that is possible. Perhaps it is possible to replace the list with a nested select statement. It really depends on the case at hand.

If that's not possible, I would put together the SQL command in my code. With a variable amount of parameters, I would simply use a loop to add the parameter placeholders to the SQL string and the parameters themselves to the command object. Taken to the extreme, this can become very ugly and results in absolutely unmaintainable code. In my current project here at work some genius tried to replace the complete data access layer with one single function which constructs every needed SQL statement in a few thousand (!) lines of spaghetti code. But, like with everything, there is no problem as long as it is used sparingly and with at least some thought.

Edit: Here is the beginning of that monster function, a worthy candidate for this board as well:

public static DataSet ArtikelHelpListe(int auswahl, string artnr, string artbez, string lnam, 
                                       string lnr, string lartnr, string wg, string mkenz,
                                       string ean, string gab, bool ges, bool bug, bool bau,
                                       bool eks, int stanort, string uid)
{
    // Here be dragons ( thousands of lines of totally worthless code)
    ....
}

A while ago he asked me what he should have printed on my business cards. I said 'Wizard'.

I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.

Re: String.Format???

Jason Christian22-Jul-10 9:05

Jason Christian

22-Jul-10 9:05

A for(each) loop to create the parameters and another to load the values works pretty well. Assuming the list of values is in some form of collection.

Re: String.Format???

PIEBALDconsult9-Jul-10 5:09

PIEBALDconsult

9-Jul-10 5:09

It isn't; pay no attention to the stored procedure fan boys.

Re: String.Format???

oggenok649-Jul-10 5:35

oggenok64

9-Jul-10 5:35

While we are at it, could someone please explain to me why inline SQL might be more vulnerable to SQL injection than a stored proc. Having thought about it all afternoon, i must admit that i just can't see it. Am i blind?

Re: String.Format???

PIEBALDconsult9-Jul-10 5:40

PIEBALDconsult

9-Jul-10 5:40

It really isn't. However, when using stored procedures, you are pretty much forced to use parameters. When using embedded SQL, you have the option, and they who don't know about parameters or are too lazy to bother, wind up with bad code.

Basically, whichever way you store your SQL statements, use parameters.

A properly-written Data Access Layer will hide the details anyway.

Re: String.Format???

Steve Wellens9-Jul-10 5:45

Steve Wellens

9-Jul-10 5:45

Here's a good article about SQL Injection:

http://www.mikesdotnetting.com/Article/113/Preventing-SQL-Injection-in-ASP.NET[^]

Steve Wellens

Re: String.Format???

PIEBALDconsult9-Jul-10 5:51

PIEBALDconsult

9-Jul-10 5:51

Yes, that's good.

Re: String.Format???

Single Step Debugger9-Jul-10 6:16

Single Step Debugger

9-Jul-10 6:16

This looks like a direct port from MFC C++ codebase to .NET and C# from someone not quite familiar with the C#.

The narrow specialist in the broad sense of the word is a complete idiot in the narrow sense of the word.

Advertise here – minimum three posts per day are guaranteed.

Re: String.Format???

M@dHatter11-Jul-10 10:33

M@dHatter

11-Jul-10 10:33

String.Format is a nice feature, but slower than just appending.

"I do not know with what weapons World War 3 will be fought, but World War 4 will be fought with sticks and stones." Einstein

"Few things are harder to put up with than the annoyance of a good example." Mark Twain

Re: String.Format???

Not Active11-Jul-10 10:53

Not Active

11-Jul-10 10:53

VectorX wrote:
just appending

String concatenation? Dead | X|

I know the language. I've read a book. - _Madmatt

Re: String.Format???

PIEBALDconsult11-Jul-10 14:55

PIEBALDconsult

11-Jul-10 14:55

It's (almost) the right tool for this job.

Re: String.Format???

Kunal Chowdhury «IN»17-Jul-10 18:45

Kunal Chowdhury «IN»

17-Jul-10 18:45

I also found in several codes (written by some experts Sniff | :^)

though) using string.Replace() to replace the text "{0}" with proper string values. Not only that, I found some code where they used:

string myStr2 = myStr1.ToString();

What is the significance of that code? Confused | :confused:

Why do you need to ToString() a string?

If experts are doing like that, then what the freshers will do, who follows that expert who has several years of experience in that field!!!

Don't forget to Click on [Vote] and [Good Answer] on the posts that helped you.

Re: String.Format???

Richard Deeming22-Jul-10 2:39

Richard Deeming

22-Jul-10 2:39

string.Format is not a replacement for parameterized queries. You probably won't get SQL Injection with integer and date parameters, but as soon as you try to use Format / Replace for string variables, little Bobby Tables[^] comes along and ruins everything.

Also, DateTime.ToShortDateString depends on both the current culture and the user's regional settings. Changing either could break your query. If you're still ~~stupid~~ brave enough to use string.Format, at least make sure you're using a custom format string and the invariant culture!

"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer

Superman if-clause

freakyit8-Jul-10 3:40

freakyit

8-Jul-10 3:40

just looked into the code of an project made from a workmate.
may be he thought superman would fly in and delete the folder between the two if-clauses xD

if (System.IO.Directory.Exists(dest))
{
    if (!System.IO.Directory.Exists(dest))
    {
      System.IO.Directory.CreateDirectory(dest);
    }

    fullDest = dest + fileInfo.Name; // what is if Directory seperator is missing? xD
}

Re: Superman if-clause

Keith Barrow8-Jul-10 8:03

Keith Barrow

8-Jul-10 8:03

Epic fail. Written at 4am by any chance Smile | :)

Dalek Dave: There are many words that some find offensive, Homosexuality, Alcoholism, Religion, Visual Basic, Manchester United, Butter.
Pete o'Hanlon: If it wasn't insulting tools, I'd say you were dumber than a bag of spanners.

Re: Superman if-clause

Kevin Drzycimski8-Jul-10 8:51

Kevin Drzycimski

8-Jul-10 8:51

freakyit wrote:
may be he thought superman would fly in and delete the folder between the two if-clauses

this could actually happen in a multi threaded environment

but then he would fly by again after the creation and delete it again =/

Re: Superman if-clause

oggenok648-Jul-10 22:12

oggenok64

8-Jul-10 22:12

Looks like a classic example of code that originally did more than this. After the "more" was moved to somewhere else, this snippet was what was left behind. I've seen many examples and they always make the original programmer look really stupid. Have to admit - I've even done it myself Smile | :)

Re: Superman if-clause

Rob Grainger8-Jul-10 22:25

Rob Grainger

8-Jul-10 22:25

Shurely,

string IsSuperman() {
    if (IsItABird())
        return "Bird";
    else if (IsItAPlane())
        return "Plane";
    else
        return "Superman";
}

(Sorry - its a bad job, but someone's gotta do it.)

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.