Thankfully, This Employee Can't Harm Our Codebase Anymore

AspDotNetDev

I was just going through an ex-coworker's code, and came across a function, "GetList", that goes through the files in a directory and returns a string. What is in the string, you ask? A friggin semicolon-separated list of file names. :| The code that calls that function then splits the string on semicolons and stores that to an array, which it then processes. I can only guess that my coworker wrote this before he felt comfortable working with collection data structures (and I'm guessing it was also upgraded from an older version of VB). Let's hope nobody puts a semicolon in their filenames. :doh: For your viewing pleasure, here is the function (with some things changed to protect the innocent):

Private Function GetList(ByVal sDir As String) As String
Dim oFile As System.IO.DirectoryInfo
oFile = New IO.DirectoryInfo(sDir)
Dim diar1 As IO.FileInfo() = oFile.GetFiles()
Dim dra As IO.FileInfo
Dim sFList As String
Dim sName As String

sFList = ""

For Each dra In diar1
    sName = System.Convert.ToString(dra)
    If MeetsCriteria(sName) Then
        sFList = sFList & sName & ";"
    End If
Next

If Len(sFList) > 0 Then
    sFList = Microsoft.VisualBasic.Left(sFList, (Len(sFList) - 1))
End If

Return sFList

End Function

I particularly like the interesting use of System.Convert.ToString. :((

Thou mewling ill-breeding pignut!

LloydA111

I don't really do .NET programming, in fact, I don't program for .NET at all, so this will probably seem like a daft question.

AspDotNetDev wrote:

I particularly like the interesting use of System.Convert.ToString.

What should have he used instead?

See if you can crack this: b749f6c269a746243debc6488046e33f
So far, no one seems to have cracked this!

The unofficial awesome history of Code Project's Bob! "People demand freedom of speech to make up for the freedom of thought which they avoid."

AspDotNetDev

dra.Name

Thou mewling ill-breeding pignut!

LloydA111

Oh I see, dra being an object.

See if you can crack this: b749f6c269a746243debc6488046e33f
So far, no one seems to have cracked this!

The unofficial awesome history of Code Project's Bob! "People demand freedom of speech to make up for the freedom of thought which they avoid."

M dHatter

Visual Basic eeeewww thats bad enough.

Septimus Hedgehog

Are you suggesting that the use of VB has self-harming potential built-in by default? ;) Something on the lines of Munchausen.Net?

"I do not have to forgive my enemies, I have had them all shot." — Ramón Maria Narváez (1800-68).

Sander Rossel

I was thinking this code wasn't THAT bad... Then I realized ; is a semi-colon and not |. Since | is not allowed in a filename on a windows machine that would've made some sense. I sometimes use semi-colons in filenames, actually always to replace the : which is also forbidden (I can only guess why). I would rename the function to GetFilenamesOfFilesInAFolderInASingleStringSeperatadBySemicolons. Now there will be some smartasses who will ask how you're going to put that in a Single or if a SingleString is something like a String that must be numeric and has a range from -3.402823e38 to 3.402823e38, but we can safely ignore these people :) Also change the semicolon to | in both the function and calling code. You have just made the code a whole lot better and safer for windows systems. Another idea, which is completely absurd and which you could never do because you can't change (or dare to change) working code and there is also no time to actually do this, but here it goes... Delete the function and write a new one that actually returns an IEnumerable(Of String). I know it's dangerous, very risky. I would comment the original function so you can always go back in case your new function doesn't work. Don't trust on source control with this one, commenting is a lot safer! Also, you should discuss this with your manager. Of course you can't just go and change existing, working code around now, can you!? :rolleyes:

It's an OO world.

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

Konstardiy from Tbilisi

+9000E9000 I totally agree.

jsc42

Naerling wrote:

sometimes use semi-colons in filenames, actually always to replace the : which is also forbidden (I can only guess why).

':' is the separator between device and the file on the device. The device (in standard MS-DOS) can be 1 or 3 chars, most obviously in filenames like C:\AUTOEXEC.BAT, but also in filenames like NUL:*.* .

Daryl Shockey

I like the added touch of trimming the last semicolon with Microsoft.VisualBasic.Left(...). Apparently it didn't occur to them to just precede each name after the first with a semicolon. (If sFList.Length > 0 Then sFList = SFList & ";")

Fabio Franco

PHS241 wrote:

VB has self-harming potential built-in by default?

I couldn't have put it better.

To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson ---- Our heads are round so our thoughts can change direction - Francis Picabia

RafagaX

It could have been worse, he migth have returned a thousand positions array of files just to get the name later. Also i understand him to some extent, because i have done similar things when i was younger and unexperienced (in PHP).

CEO at: - Rafaga Systems - Para Facturas - Modern Components for the moment...

Billy T

> I would rename the function to GetFilenamesOfFilesInAFolderInASingleStringSeperatadBySemicolons. Then you'd have people laughing at your spelling...

Sander Rossel

Nothing wrong with my spelling :)

It's an OO world.

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

Mark Puddephat

That's what you get for using Microsoft, man. Try Linux for size... getList() {ls -1 2>/dev/null | paste -d\; -s -} myListOfFiles=$(getList) Failing that, let me introduce you to my friend who was eventually fired a decade ago because of the quality of his work. We are still living with the legacy of his bad code!

Paulo_JCG

Quote:

I know it's dangerous, very risky. I would comment the original function so you can always go back in case your new function doesn't work. Don't trust on source control with this one, commenting is a lot safer! Also, you should discuss this with your manager. Of course you can't just go and change existing, working code around now, can you!?

<Obsolete("You MUST not use this routine any more. Use my BeautifullOne instead.", True)> does miracles ;P

Paulo Gomes Over and Out :D

Sander Rossel

[Obsolete("Don't use this application, it was written by morons!")]
static class Program
{
/// <summary>
/// The main entry point for the application.
/// </summary>
[STAThread]
static void Main()
{
Application.EnableVisualStyles();
Application.SetCompatibleTextRenderingDefault(false);
Application.Run(new MainForm());
}
}

;P

It's an OO world.

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