Thankfully, This Employee Can't Harm Our Codebase Anymore
-
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 StringsFList = "" 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 sFListEnd Function
I particularly like the interesting use of System.Convert.ToString. :((
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 aSingleor if aSingleStringis something like aStringthat 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 anIEnumerable(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(); }
} -
+9000E9000 I totally agree.
-
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 aSingleor if aSingleStringis something like aStringthat 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 anIEnumerable(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(); }
}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:*.* .
-
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 StringsFList = "" 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 sFListEnd Function
I particularly like the interesting use of System.Convert.ToString. :((
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 & ";")
-
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).
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
-
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 StringsFList = "" 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 sFListEnd Function
I particularly like the interesting use of System.Convert.ToString. :((
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...
-
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 aSingleor if aSingleStringis something like aStringthat 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 anIEnumerable(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(); }
} -
> I would rename the function to
GetFilenamesOfFilesInAFolderInASingleStringSeperatadBySemicolons. Then you'd have people laughing at your spelling...Nothing wrong with my spelling :)
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
} -
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 StringsFList = "" 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 sFListEnd Function
I particularly like the interesting use of System.Convert.ToString. :((
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!
-
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 aSingleor if aSingleStringis something like aStringthat 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 anIEnumerable(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(); }
}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
-
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
[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(); }
}
