|
I'm no good at Big O notation, but that's some kind of amortized N⁵, isn't it?
|
|
|
|
|
1. This code was COPIED to 3 different places - they didn't bother to make a function out of all of it. That is probably the worst error, because now if this 'new' code has bugs, you have to modify three places instead of one.
2. It 'excessively' calculates the length of the string. I think I am being kind here when I say that.
3. 'Familiarity' was my joke - there is already a function in Windows to do this.
I need a 32 bit unsigned value just to hold the number of coding WTF I see in a day …
|
|
|
|
|
Blake Miller wrote: if this 'new' code has bugs, you have to modify three places instead of one.
Or you can create a function with this code and replace the code in the other 3 (and modify just one). 
|
|
|
|
|
Blake Miller wrote: 2. It 'excessively' calculates the length of the string.
And just as excessively casts wName to LPSTR.
|
|
|
|
|
Not sure if you are joking, or looking for a serious answer.
The code you posted makes WAY too many passes through the source string. If you're looking for maximum performance, you can get this down to just a single pass thusly:
LPSTR* nxt = wName;
while(*nxt && isspace(*nxt))
nxt++;
if (!*np) {
wName = '\0';
return wName;
}
LPSTR* start = nxt;
LPSTR* last_no_ws = nxt;
while(*nxt) {
if (!isspace(*nxt))
last_no_ws = nxt;
nxt++;
}
int len = (last_no_ws - start) + 1;
memmove(wName, start, len);
wName[len] = '\0';
return wName;
You could even save the 'memove' at the end by just modifying the wName string in place (add null after last_no_ws), then return the reference to 'start'.
|
|
|
|
|
This is great. I was just posting the orginal code because of the three reasons I listed. 1. Should be a function - REGARDLESS. 2 - It's inefficient on multiple levels. 3. MS already has the 'tried and true' function to call anyways.
I like to see that others do not consider what I posted to be optimal in ANY case, so that is comforting
I need a 32 bit unsigned value just to hold the number of coding WTF I see in a day …
|
|
|
|
|
Changes:
1. Clear defined end of first loop cutting trailing spaces
2. No undefined behavior of strcpy with overlapping memory areas
3. No unnecessary strlen calls
4. Unified way to test for whitespaces via isspace
5. No warnings because of signed/unsigned conflicts (int <-> unsigned int strlen(...))
LPSTR myStrTrim(LPSTR pszString)
{
LPSTR pszLastChar = pszString + strlen(pszString) - 1;
while (pszLastChar >= pszString && isspace(*pszLastChar))
*pszLastChar-- = '\0';
LPSTR pszFirstChar = pszString;
while(pszFirstChar < pszLastChar && isspace(*pszFirstChar))
++pszFirstChar;
memmove(pszString, pszFirstChar, pszLastChar-pszFirstChar+1);
return pszString;
}
|
|
|
|
|
One of the best languages for string manipulation is PowerBasic. You can use it to write 32 bit DLL's callable from other languages.
To trim leading spaces: LTRIM$(AnyStringVar)
To trim any character from left: LTRIM$(AnyStringVar, SomeCharacter)
To trim any of multiple characters from left: LTRIM$(AnyStringVar, ANY SomeCharacters)
To trim trailing edge spaces: RTRIM$(AnyStringVar)
To trim any trailing character: RTRIM$(AnyStringVar,SomeCharacter)
To trim any of multiple trailing characters: RTRIM$(AnyStringVar, ANY SomeCharacters)
Trim both leading and trailing spaces: TRIM$(AnyStringVar)
Trim any character leading or trailing: TRIM$(AnyStringVar, SomeCharacter)
Trim any of multiple characters leading and trailing: TRIM$(AnyStringvar, ANY SomeCharacters)
PowerBasic supports pointers too !
PowerBasic has many other powerful string functions (both ANSI and UNICODE) such as:
ACODE$ - Translate a Unicode string into an ANSI string.
BIN$ - Return a string with the binary (base 2) representation of a value.
BITS$ - Copies string contents without modification.
BUILD$ - Concatenate multiple strings with high efficiency.
CHOOSE$ - Return one of several values, based upon the value of an index.
CHR$ - Convert one or more character codes into ASCII character(s).
CHR$$ - Convert one or more character codes into Unicode character(s).
CHRBYTES - Determine the size of a single character in a string variable.
ChrToOem$ - Translates a string of ANSI/WIDE characters to OEM byte characters.
ChrToUtf8$ - Translates a string of ANSI/WIDE characters to UTF-8 byte characters.
CLIP$ - Deletes characters from a string.
CLSID$ - Return a 16-byte (128-bit) GUID string containing a CLSID.
CSET - Center a string within the space of another string or UDT.
CSET$ - Return a string containing a centered (padded) string.
DEC$ - Convert an integral value to a decimal string.
EXTRACT$ - Return up to the first occurrence of a specified character.
FORMAT$ - Return a string containing formatted numeric data.
GUID$ - Return a 16-byte (128-bit) Globally Unique Identifier GUID.
GUIDTXT$ - Return a 38-byte human-readable GUID/UUID string.
HEX$ - Hexadecimal (base 16) string representation of an argument.
INSTR - Search a string for the first occurrence of a character or string.
JOIN$ - Return a string consisting of all of the strings in a string array.
LCASE$ - Return a lowercase version of a string argument.
LEFT$ - Return the left-most n characters of a string.
LEN - Return the logical length of a variable, UDT, or Union.
LSET - Left-align a string within the space of another string or UDT.
LSET$ - Return a string containing a left-justified (padded) string.
MAX$ - Return the argument with the largest (maximum) value.
MCASE$ - Return a mixed case version of a string argument.
MID$ - Return a portion of a string.
MID$ - Replace characters in a string with characters from another string.
MIN$ - Return the argument with the smallest (minimum) value.
MKBYT$ - Convert a Byte value into a binary encoded string.
MKCUR$ - Convert a Currency value into a binary encoded string.
MKCUX$ - Convert an Extended Currency value into a binary encoded string.
MKD$ - Convert a Double-precision value into a binary encoded string.
MKDWD$ - Convert a Double-word value into a binary encoded string.
MKE$ - Convert an Extended-precision value into a binary encoded string.
MKI$ - Convert a integral value into a binary encoded string.
MKL$ - Convert a Long-integer value into a binary encoded string.
MKQ$ - Convert a Quad-integer value into a binary encoded string.
MKS$ - Convert a Single-precision value into a binary encoded string.
MKWRD$ - Convert a Word value into a binary encoded string.
NUL$ - Return a string containing a specified number of $NUL characters.
OCT$ - Return a string that is a octal (base 8) representation of a value.
OemToChr$ - Translates a byte string of OEM characters into ANSI/WIDE characters.
PARSE - Parse a string and extract all delimited fields into an array.
PARSE$ - Return a delimited field from a string expression.
PARSECOUNT - Return the count of delimited fields in a string expression.
PEEK$ - Returns consecutive 1-byte characters starting at a specific memory location.
PEEK$$ - Returns consecutive 2-byte wide characters starting at a specific memory location.
POKE$ - Store a sequence of bytes starting at a specific memory location.
POKE$$ - Store a sequence as 2-byte wide characters starting at a specific memory location.
PROGID$ - Return the alphanumeric PROGID string (text) of a given CLSID.
REGEXPR - Scan a string for a matching "wildcard" or regular expression.
REGREPL - Scan a "wildcard" match in a string with a new string.
REMAIN$ - Returns the portion of a string which follows the first occurrence of a character or group of characters.
REMOVE$ - Return a copy of a string with characters or strings removed.
REPEAT$ - Return a string consisting of multiple copies of a specified string.
REPLACE - Replace all occurrences of one string with another string.
RETAIN$ - Return a string with all non-specified characters removed.
RIGHT$ - Return the rightmost n characters of a string.
RSET - Right justify a string into the space of a string variable or UDT.
RSET$ - Return a string containing a right-justified (padded) string.
SHRINK$ - Shrinks a string to use a consistent single character delimiter.
SIZEOF - Return the total or physical length of any PowerBASIC variable.
SPACE$ - Return a string consisting of a specified number of spaces.
SPLIT - Splits a string into two parts.
STR$ - Return the string representation of a number in printable form.
STRDELETE$ - Delete a specified number of characters from a string expression.
STRING$ - Returns an ANSI string consisting of multiple copies of a specified character.
STRING$$ - Returns a WIDE string consisting of multiple copies of a specified character.
STRINSERT$ - Insert a string at a specified position within another string.
STRPTR - Return the address of the data held by a variable length string.
STRREVERSE$ - Reverse the contents of a string expression.
SWAP - Exchange the values of two strings, pointers, or pointer targets.
SWITCH$ - Return one item of a series based upon a True/False evaluation.
TAB$ - Return a string with TAB characters expanded with spaces.
TALLY - Count the number of occurrences of specified characters/strings.
UCASE$ - Return an all-uppercase (capitalized) version of a string.
UCODE$ - Translate an ANSI string into a Unicode string.
UNWRAP$ - Removes paired characters from the beginning and end of a string.
USING$ - Format string/numeric expressions using a mask string.
Utf8ToChr$ - Translates a byte string of OEM characters into ANSI/WIDE characters.
VERIFY - Determine if each character of a string is in another string.
WRAP$ - Adds paired characters to the beginning and end of a string.
PowerBasic string functions are fast, fast, fast. The compiler was written in assembler and the generated machine code is optimized for speed.
|
|
|
|
|
Is that supposed to be a (long) joke or is it just stupid spam?
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
I feel your pain - points to your signature.
|
|
|
|
|
[Edit]
I know it is not a crime, but I had to fight against inconsistent variable names and bit of spaghetti code.
But the most important thing is: This piece of code has to parse a huge amount of data (up to 500000 rows with parameters and so on) so you remember if it needs 50 or 4 seconds to parse the parameter values (overall time).
[Edit end]
I found this piece of code in a project which I had to expand. Notice the guy who wrote the code was at the end of his apprenticeship with already 4 years experience in coding.  
public void LogFileBreakDown(string channel, string channelnumber)
{
int channelnumber = 0;
try
{
channelnumber = Int32.Parse(channelnumber);
}
catch(Exception ex)
{
channelnumber = 0;
}
return;
}
I replaced Parse with TryParse -> and the log file was loading about 30 times faster then before.
[EDIT: Added the new source as it is now]
public string Aufschluesselung_sys(String logFileLineText, String channelNumber, String channelName, bool showWarnings, ref string warnings, CultureInfo messageLanguage)
{
string sName, sID, sSplit = logFileLineText;
string[] mySplit = sSplit.Split(new Char[] { '>', '<', '"' });
string ctrlCommonSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlCommonSpecificationXml"]);
string cryptoModuleSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCryptoModuleSpecificationXml"]);
string ctrlDcSlaveSpecificationXml = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlDcSlaveSpecificationXml"]);
string ctrlIseSpecification = Path.Combine(Path.GetDirectoryName(System.Reflection.Assembly.GetExecutingAssembly().GetName().CodeBase), ConfigurationManager.AppSettings["PathToCtrlISESpecificationXml"]);
XmlDocument XmlAufschluesselung = new XmlDocument();
int iID;
if (mySplit.Count() >= 2)
{
if (Int32.TryParse((mySplit[1]), out iID))
{
if (Suche.Suchen(logFileLineText, (channelNumber + ">")) || Suche.Suchen(logFileLineText, (channelNumber + "<")))
{
if (iID >= 1 && iID <= 102)
{
XmlAufschluesselung.Load(ctrlCommonSpecification);
}
else
{
if (iID == 257)
{
XmlAufschluesselung.Load(cryptoModuleSpecification);
}
else
{
if (iID >= 1001 && iID <= 4228)
{
XmlAufschluesselung.Load(ctrlDcSlaveSpecificationXml);
}
else
{
if (iID >= 4288 && iID <= 4565)
{
XmlAufschluesselung.Load(ctrlIseSpecification);
}
}
}
}
XmlNode controllerNode = XmlAufschluesselung.SelectSingleNode("Controller");
if (controllerNode != null)
{
foreach (XmlNode xmlNode in controllerNode.ChildNodes)
{
if (xmlNode.Name.Equals("Command"))
{
sName = xmlNode.Attributes["name"].Value.ToString();
sID = xmlNode.Attributes["id"].Value.ToString();
if (sName == "RebootICSoftware")
{
Console.WriteLine("FOUND");
}
if (iID.ToString() == sID)
{
mySplit[1] = mySplit[1].Replace(iID.ToString(), sName);
}
}
}
}
else
{
ResourceManager translator = new ResourceManager("LogFileInterpreter.resource_language", typeof(LogfileBreakDown_C111).Assembly);
string warningMessage = translator.GetString("msgNotAbleToResolveCommandNameFromXmlByCommandId", messageLanguage);
WriteWarningLog(translator.GetString("msgNotAbleToResolveCommandNameFromXmlByCommandId", new CultureInfo("EN")) + ";" + logFileLineText);
warnings = "WARNING:\n" + warningMessage + "\nLine text: " + logFileLineText + "\n\n" + warnings;
if (showWarnings)
{
ShowMessageForm showMessageForm = new ShowMessageForm(MessageType.Warning, "WARNING:\n" + warningMessage + "\n\n " + logFileLineText);
showMessageForm.ShowDialog();
}
}
logFileLineText = logFileLineText.Replace(channelNumber, channelName) + mySplit[1] + '"' + mySplit[2] + '"';
}
}
}
return logFileLineText;
}
Intelligence is not like IntelliSense
modified 1-Oct-12 3:05am.
|
|
|
|
|
Was this taken from old .NET 1/1.1 code though? TryParse wasn't introduced until .NET 2.
|
|
|
|
|
It was made with .Net Framework 4.0, So TryParse was definately avaible. I didn't even had to change the target framework specification in the VS Setup.... 
|
|
|
|
|
Which is why I wrote my own utility method, TryParseInt, back in 2001...
The method was based on the primary school teaching "4321" = 1 + 2*10 + 3*100 + 4*1000. An ad-hoc attempt to recreate it (although it's kinda pointless now)
static public bool TryParseInt(string s, out int n)
{
int baseValue = 1;
n = 0;
try
{
for (int pos = s.Length; pos >= 0; pos--)
{
char c = s[pos];
if (c == '-')
{
if (pos == 0)
n *= -1;
else
return false;
}
else if (c <= '0' || c >= '9')
return false;
n += baseValue * (int)(c - '0');
}
}
catch (OverflowException)
{
return false;
}
return true;
}
Funnily enough, when int.Parse was introduced with FW 2.0, I tested a little for fun, expecting the FW method to far and away outperform my simplistic method. But instead it didn't perform nearly as well as my simplistic method.
Of course mine didn't bother about cultures or thousand separators, knew nothing about number formats, only worked for "invariant" (i.e. US!) culture, and even fell back on a try-catch to handle overflow. Still, for our use - where we'd interpret a string as one thing if it was a valid int, otherwise as soemthing else - this worked very well. 
|
|
|
|
|
How did he get it to compile?
A local variable named 'channelnumber' cannot be declared in this scope because it would give a different meaning to 'channelnumber', which is already used in a 'parent or current' scope to denote something else
Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water
|
|
|
|
|
Yes, This was a writing mistake by myself. The scond variables name should be "id". Sorry for that.
|
|
|
|
|
Marco Bertschi wrote: The scond variables name should be "id".
That was the first thing I saw, the second was the code didn't do anything. Third, I learned something. I had no idea Parse was slower than TryParse and I wouldn't have found out about it because the user is either a devious B or an idiot and won't put a number where asked to do so, so I wouldn't use Parse a second later than when I found out about TryParse. I also thought that a parsing routine should be as fast for both. Then I thought, it wouldn't be that fast if Parse was hitting exceptions, it gets caught miles from where it blew up and takes a while to get back to doing its job. Was it slow because it was blowing up or is there some intrinsic problem with the routine? (Besides being willing to blow up at the drop of a "h" at.)
"Doh", I shouldn't type while thinking has gone out the window. Forget everything above, I probably have forgotten it already.
|
|
|
|
|
KP Lee
It does something, but I removed the specific code under the Parsing part.
It was not slow because it threw an exception. It was slow because it threw about 500 exceptions during the import of one single file.
However, your are right when you say that only a complete moron of a user would put a string instead of a number when a number is required.
To complete the explanation, I added the whole source code to the original post.
|
|
|
|
|
Parse is not significantly slower than TryParse. (I suspect if you have a look with Reflector, you'll find it uses TryParse under the hood. Academically speaking I suppose it must in that case be slower, but in practice the difference would be insignificant at best.)
But Parse in conjunction with try-catch is obviously a LOT slower than TryParse in conjunction with "if". And it's not necessary for the stack to be deep for this to be the case. Throwing an exception means allocating a new object (the exception) on the heap and this alone makes it far more costly. If you experiment a little with a simple console or winforms app you'll note that the first time you throw and catch an exception incurs a noticeable delay. After that it is a lot faster, but still thousands of times slower than returning a bool.
|
|
|
|
|
He didn't know about TryParse. I don't think that's a crime, really, particularly if not much of that 4 years' experience was in recent .Net. What he's done is exactly the pattern that you had to use before, and still have to use in Java.
|
|
|
|
|
Dear Bob
A good argument indeed. I edited my original post with more information because there came up the whole discussion with performance and "How user-friendly is the program" for us.
And the program used to crahs because it hung up in the whole try-catch story, so at least then he should've been looking for a faster way, doesn't he 
|
|
|
|
|
Well if he's expecting a conversion/parsing exception, he still shouldn't be doing that Pokemon thing
Swallowing exceptions can be very dangerous sometimes, especially with large codebases and large input files.
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
True in principle ... but in reality there's no other exception that's going to be thrown in there, so catching Exception instead of FormatException doesn't really offend me in this case.
|
|
|
|
|
OutOfMemoryException and ThreadAbortException come to mind...
Again, it's a big, long process (OP mentioned 50 seconds).
'I'm French! Why do you think I've got this outrrrrageous accent?' Monty Python and the Holy Grail
|
|
|
|
|
I'd say "it depends". If the code was performance-critical, he could have written his own rudimentary method to avoid relying on exceptions to find out if a string represents a valid integer or not. He *should* know that throw-catch is slow, even if he's never seen a TryParse method before.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
