Ugly Lines of Code
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
To me there is a threshhold. Take #1 for example. That doesn't bother me. But if we had a few more likes of tab_data [i], I probably would use a variable pointer incrementing through the array. #2? That is too much for me. I would us a temporary variable. The important thing to remember is that with today's compilers, making human readable code is much more important that false hand optimizations. #3? Ehhh, I never liked using the return value of a function directly if it was any thing more complicated than a simple value. Tim Smith Descartes Systems Sciences, Inc.
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
I would do all of the above without breaking them up. Christian After all, there's nothing wrong with an elite as long as I'm allowed to be part of it!! - Mike Burston Oct 23, 2001
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
And you consider these ugly? You should see some of the SQL request formatting (as string) that is going on in here... ;) Seriously thought your examples are not that much ugly as they are dangerous. You should always error-check the pointer values or it will crash on you. Then it will not only be short lined but also more reliable...
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
It's harder to debug when you include everything on one line. I like to break my stuff up now and write things like bool bLoadFile = LoadFile(filename); ASSERT(bLoadFile); if (!bLoadFile) { LogError("blah"); return; } instead of if (!LoadFile(filename)) return; It takes more code but it sure makes debugging 1000x faster. I also like to make my function names as descriptive as possible. Instead of TabData(num).pTab->SetActive(); I might write CTabData* pTabData = GetTabData(num); ASSERT(pTabData); pTabData->SetActive(); It takes a few more seconds to type that but if it ever crashes because of a bad pointer then it's going to take you a lot longer to figure out why and where.
Todd Smith
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
Adam Arthur wrote: if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); Can you 100% be sure, that
tab_data[i].pTabis valid, iftab_data[i].tab_num!= -1 ? I would add some runtime checks, which are in debug builds only, just to be sure:if(tab_data[i].tab_num != -1)
{
ASSERT (tab_data[i].pTab);
ASSERT (tab_data[i].pTab->GetSafeHwnd());
tab_data[i].pTab->DestroyWindow();
}ASSERT (pMainPointer);
ASSERT (pMainPointer->GetCurTab());
DlgData(pMainPointer->GetCurTab())->OnKillActive();Regards Thomas
Disclaimer:
Because of heavy processing requirements, we are currently using some of your unused brain capacity for backup processing. Please ignore any hallucinations, voices or unusual dreams you may experience. Please avoid concentration-intensive tasks until further notice. Thank you. -
It's harder to debug when you include everything on one line. I like to break my stuff up now and write things like bool bLoadFile = LoadFile(filename); ASSERT(bLoadFile); if (!bLoadFile) { LogError("blah"); return; } instead of if (!LoadFile(filename)) return; It takes more code but it sure makes debugging 1000x faster. I also like to make my function names as descriptive as possible. Instead of TabData(num).pTab->SetActive(); I might write CTabData* pTabData = GetTabData(num); ASSERT(pTabData); pTabData->SetActive(); It takes a few more seconds to type that but if it ever crashes because of a bad pointer then it's going to take you a lot longer to figure out why and where.
Todd Smith
So do you also break function parameters onto seperate lines? e.g.
if(m_VarA != m_VarB) {
rect.SetRect(m_VarC*m_VarD,
(20*VarE)-20,
(m_VarC*VarE)+(m_VarC*m_VarD),
((20*VarC)-20)+m_VarD);
FunctionA(pDC,
rect,
FunctionB(pDoc->m_dataSet.m_Member, true),
FunctionB(pDoc->m_dataSet.m_Member, false),
bParamFour);
}Obviously, with comments after each parameter to assist with debugging. The last time i did this was when i was programming Windows without MFC. Splitting those long function calls for the window procedures etc seemed quite necessary then. How many people do this as a habit? Or do your employers make you do it as a consistent coding style? Simon Hey, it looks like you're writing a letter! Sonork ID 100.10024
-
So do you also break function parameters onto seperate lines? e.g.
if(m_VarA != m_VarB) {
rect.SetRect(m_VarC*m_VarD,
(20*VarE)-20,
(m_VarC*VarE)+(m_VarC*m_VarD),
((20*VarC)-20)+m_VarD);
FunctionA(pDC,
rect,
FunctionB(pDoc->m_dataSet.m_Member, true),
FunctionB(pDoc->m_dataSet.m_Member, false),
bParamFour);
}Obviously, with comments after each parameter to assist with debugging. The last time i did this was when i was programming Windows without MFC. Splitting those long function calls for the window procedures etc seemed quite necessary then. How many people do this as a habit? Or do your employers make you do it as a consistent coding style? Simon Hey, it looks like you're writing a letter! Sonork ID 100.10024
Whenever a line is wider than my window (generally about 120 characters), I split it. I hate being forced to scroll horizontally in the source code window. Regards Thomas
Disclaimer:
Because of heavy processing requirements, we are currently using some of your unused brain capacity for backup processing. Please ignore any hallucinations, voices or unusual dreams you may experience. Please avoid concentration-intensive tasks until further notice. Thank you. -
It's harder to debug when you include everything on one line. I like to break my stuff up now and write things like bool bLoadFile = LoadFile(filename); ASSERT(bLoadFile); if (!bLoadFile) { LogError("blah"); return; } instead of if (!LoadFile(filename)) return; It takes more code but it sure makes debugging 1000x faster. I also like to make my function names as descriptive as possible. Instead of TabData(num).pTab->SetActive(); I might write CTabData* pTabData = GetTabData(num); ASSERT(pTabData); pTabData->SetActive(); It takes a few more seconds to type that but if it ever crashes because of a bad pointer then it's going to take you a lot longer to figure out why and where.
Todd Smith
Todd's right. There is more to writing code than just trying to get the compiler to produce an executable. At some point, someone is going to have to look at this source, and if there is anything you can do to help them (or you) understand what you were doing, then that is well worth the time. That even goes for comments. But the extra typing to simplify debugging is also well worth it. Again, the goal is frequently to get "good" code working properly in the shortest amount of time, as well as to simplify the process of changing it later on. Dave
-
Whenever a line is wider than my window (generally about 120 characters), I split it. I hate being forced to scroll horizontally in the source code window. Regards Thomas
Disclaimer:
Because of heavy processing requirements, we are currently using some of your unused brain capacity for backup processing. Please ignore any hallucinations, voices or unusual dreams you may experience. Please avoid concentration-intensive tasks until further notice. Thank you.Thomas Freudenberg wrote: Whenever a line is wider than my window (generally about 120 characters), I split it. I hate being forced to scroll horizontally in the source code window. Me too Thomas, Shucks, It'll all compile the same. P.S I've always liked your Regardz Colin J Davies
Sonork ID 100.9197:Colin
I live in Bob's HungOut now
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
Apparently I am more obssessed than most ;) I don't like structure array addressing much, mostly from a debugging/data watching perspective. I prefer to get a pointer first : TabData *ptabdata = &tab_data[i]; I also don't like comparing with -1 very much. What about -2 ? Assuming that 0 and above are the positive logic case, I would do this : if( ptabdata->tab_num >= 0 ) if( ptabdata->pTab ) ptabdata->pTab->DestroyWindow(); If I were going to use pTab more than once I would save it in a temporary variable. Obviously, I lean toward the break-it-down side. The degree of my leaning is sometimes proportional to the time remaining until deadline. :cool:
-
Hey, I was just wondering how all of you felt about writing ugly, long lines of code. Or, do you think it is better to break what could be a single line into many for readability? These are some times that I recently wrote... I've written much uglier lines, but this is what I could find quickly... if(tab_data[i].tab_num != -1) tab_data[i].pTab->DestroyWindow(); DlgData(pMainPointer->GetCurTab())->OnKillActive(); TabData(num).pTab->SetActive(); So, do you think it's better to break these down for readability, or to keep them compact like I have done?
Very solid advice from all of you. I suppose its a trade off. I suppose the question comes down to how readable you want your code to be. A coworker of mine once gave me the worst class ever. Here is a cut & paste of what some of the code looked like: int x = 0; int y = 0; int _x = 0; int _y = 0; int __x = 0; int __y = 0; int yEdge = GetSystemMetrics(SM_CXFIXEDFRAME); int yScroll = GetSystemMetrics(SM_CXVSCROLL); int t = yEdge + yScroll; HDWP wndPos = BeginDeferWindowPos(14); CRect pRect; AfxGetMainWnd()->GetWindowRect(pRect); CRect yRect; AfxGetMainWnd()->GetClientRect(yRect); Without cluttering up space with a really long function, essentially he proceeded to use the "x" and "y" variables to calculate window positions and correctly place them on the screen. The thing was, he left the company and guess who had to maintain things?