|
Layers? Now that's a funny idea
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
Sounds like some sort of automated test cases being run one after the other.
-Prakash
|
|
|
|
|
Oh wow. Who needs method calls anyway? Goto and Gosub should be more than enough for real programmers.
Deja View - the feeling that you've seen this post before.
|
|
|
|
|
Pete O'Hanlon wrote: Goto and Gosub should be more than enough for real programmers
|
|
|
|
|
Wow, maybe you should submit that to Guinness, it may be a new world record for a page_load!
|
|
|
|
|
Sounds like a software version of a Rube Goldberg machine...
|
|
|
|
|
I disagree with what everyone said. I can think of a lot of things that would justify a long Page_Load function like that. There is nothing inherently wrong with a really long function. It usually indicates that something needs to be re-factored, but that is not always the case. If everything in the function is atomic, unique, and well-formed... then there's no reason to make it even longer by breaking it up into multiple parts, which would only serve to decrease performance by the amount of the function-calling overhead.
I would rather read an if..else structure that had 50 branches, than dive into 50 separate functions which are only called one time.
|
|
|
|
|
I hope you are kidding!
It usually indicates that it is way too complicated and needs to be broken down in to smaller chunks. Of course there are acceptations, but they are far and few between.
INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra
|
|
|
|
|
No I wasn't joking. It's rare, but a long a function isn't always wrong. If everything is only done once, why push the stack and everything else you have to do? You're right though, it usually indicates something that hasn't been thought out properly.
|
|
|
|
|
void CArchiveEnc::EncryptData(DWORD dwOffset, DWORD dwLength)
{
DWORD dwDataLen = m_lpBufCur - m_lpBufStart;
if(dwDataLen == 0 || dwDataLen < dwOffset) return;
if(dwLength ==(DWORD)-1)
dwLength = dwDataLen - dwOffset;
if(dwDataLen < dwOffset + dwLength) return;
BYTE byFoo = 0x00;
BYTE* pData = m_lpBufStart + dwOffset;
while(pData < m_lpBufCur) byFoo = *pData++ ^= byFoo;
}
The last line is the killer. Why write it readable if one can put it on a single line
codito ergo sum
|
|
|
|
|
Ask Kernighan and Ritchie or even Stroustrup why they allowed it ! Don't complain if the compiler don't...
Kochise
In Code we trust !
|
|
|
|
|
XORing a byte by the previous byte isn't a particularly great encryption system. Crackable in moments.
Also note that the parameter dwLength is basically ignored, after determining whether the requested offset and length is within the buffer. Instead, all the data up to the end of the buffer is 'encrypted'.
If forced to rewrite this rather than use a proper encryption algorithm, I would replace the last line with:
for( DWORD byte = 0; byte < dwLength; ++byte )
{
pData[byte] ^= byFoo;
byFoo = pData[byte];
} Note I'm using array indexing rather than pointer arithmetic. Converting from one to the other is a simple and fundamental optimization which all C++ compilers will implement.
|
|
|
|
|
BadKarma wrote: while(pData < m_lpBufCur) byFoo = *pData++ ^= byFoo;
They wanted to encrypt the encryption algorithm?
Cheers,
Vikram.
"But nowadays, it means nothing. Features are never frozen, development keeps happening, bugs never get fixed, and documentation is something you might find on wikipedia."
- Marc Clifton on betas. Join the CP group at NationStates. Password: byalmightybob
|
|
|
|
|
Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
bool CReportMethods::IsSalesTypeValid(const CXmlEntry & Entry)
{
if ( Entry.sty.GetValueAsLong() == 4L )
{
return(true);
}
else if ( Entry.sty.GetValueAsLong() == 5L )
{
return(true);
}
else
return(true);
}
|
|
|
|
|
Wow. I just really hope it's result of changes in sales type validity rather than that someone wrote it like this from scratch Still its just wrong on so many levels. Nice.
"Throughout human history, we have been dependent on machines to survive. Fate, it seems, is not without a sense of irony. " - Morpheus
"Real men use mspaint for writing code and notepad for designing graphics." - Anna-Jayne Metcalfe
|
|
|
|
|
I saw (Visual Basic)
IF a=1 AND a<>2 and a<>3 and a<>4 THEN ...
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
It's that case where another thread accesses a during the call..., oh wait - you said VB
Deja View - the feeling that you've seen this post before.
|
|
|
|
|
Even in a multithreaded application the code shown didn't make any sense (at least for me).
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
|
|
|
|
|
True, but I really should have put a joke icon on the post.
Deja View - the feeling that you've seen this post before.
|
|
|
|
|
surely it was coded that way to ensure extensibility?!
|
|
|
|
|
Everyones a winner, I wonder if the unit testing picked up on this.
.net is a box of never ending treasures, every day I get find another gem.
|
|
|
|
|
Of course it did. Assert.IsTrue(...); Never fails.
Deja View - the feeling that you've seen this post before.
|
|
|
|
|
Ah yes, the unit tests don't exist.
From the method comments, it seems like it should return true only for the two ifs and false for the else. I'm not even sure who to talk to about this; it was coded 3 years ago by a contractor in another country and not part of the packages we're working on.
My prediction is that this baby's here to stay.
|
|
|
|
|
to_be_defined wrote: it was coded 3 years ago by a contractor in another country
Yup, that explains all.
.net is a box of never ending treasures, every day I get find another gem.
|
|
|
|
|
to_be_defined wrote: Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
Hold on, so what you're saying is that you keep looking at a function that does a few checks and returns (true) irregardless of what it checks and what happens.....and _no-one_ has changed it to remove the checks...hmm :\
|
|
|
|