Platform SDK documentation
-
Again, the answer is maintenance. I don't agree with rigid rules, but, if you should consider whether your code will be understood by people maintaining it. If you have numerous exit points in a function, then maybe the function is doing too much :).
Well, which piece of code do you think is more maintainable. 1.
HRESULT DoSomething(int param1, int param2)
{
if (IsInvalidParam1(param1))
return E_INVALIDPARAM1;
if (IsInvalidParam2(param2))
return E_INVALIDPARAM2;
if (param1 != param2)
return E_INVALIDARGS;
//Do Actual stuff
return S_OK;
}HRESULT DoSomething(int param1, int param2)
{
HRESULT ret = S_OK;
if (IsInvalidParam1(param1))
{
ret = E_INVALIDPARAM1;
}
else if (IsInvalidParam2(param2))
{
ret = E_INVALIDPARAM2;
}
else
{
if (param1 != param2)
ret = E_INVALIDARGS;
else
{
//Do Actual stuff
}
}
return ret;
}Regards Senthil _____________________________ My Blog | My Articles | WinMacro
-
Well, which piece of code do you think is more maintainable. 1.
HRESULT DoSomething(int param1, int param2)
{
if (IsInvalidParam1(param1))
return E_INVALIDPARAM1;
if (IsInvalidParam2(param2))
return E_INVALIDPARAM2;
if (param1 != param2)
return E_INVALIDARGS;
//Do Actual stuff
return S_OK;
}HRESULT DoSomething(int param1, int param2)
{
HRESULT ret = S_OK;
if (IsInvalidParam1(param1))
{
ret = E_INVALIDPARAM1;
}
else if (IsInvalidParam2(param2))
{
ret = E_INVALIDPARAM2;
}
else
{
if (param1 != param2)
ret = E_INVALIDARGS;
else
{
//Do Actual stuff
}
}
return ret;
}Regards Senthil _____________________________ My Blog | My Articles | WinMacro
S. Senthil Kumar wrote:
which piece of code do you think is more maintainable
The first one.
Maxwell Chen
-
Well, which piece of code do you think is more maintainable. 1.
HRESULT DoSomething(int param1, int param2)
{
if (IsInvalidParam1(param1))
return E_INVALIDPARAM1;
if (IsInvalidParam2(param2))
return E_INVALIDPARAM2;
if (param1 != param2)
return E_INVALIDARGS;
//Do Actual stuff
return S_OK;
}HRESULT DoSomething(int param1, int param2)
{
HRESULT ret = S_OK;
if (IsInvalidParam1(param1))
{
ret = E_INVALIDPARAM1;
}
else if (IsInvalidParam2(param2))
{
ret = E_INVALIDPARAM2;
}
else
{
if (param1 != param2)
ret = E_INVALIDARGS;
else
{
//Do Actual stuff
}
}
return ret;
}Regards Senthil _____________________________ My Blog | My Articles | WinMacro
That is not a suitable piece of code to illustrate the perils of multiple returns. All the returns are orderly and on the same nesting level, and gets done before the intended work of the function begins. So, (1) is an obvious choice. I already stated that I am not an absolutist, and does not favour any rigid rules. What if there are returns littered in the "Do Actual stuff" also. When you have returns sprinked in a piece of code at different nesting levels, it can be quite hard to understand. Thomas
-
That is not a suitable piece of code to illustrate the perils of multiple returns. All the returns are orderly and on the same nesting level, and gets done before the intended work of the function begins. So, (1) is an obvious choice. I already stated that I am not an absolutist, and does not favour any rigid rules. What if there are returns littered in the "Do Actual stuff" also. When you have returns sprinked in a piece of code at different nesting levels, it can be quite hard to understand. Thomas
Thomas George wrote:
I already stated that I am not an absolutist, and does not favour any rigid rules.
Me too, I admit that returns at different nesting levels can be hard to understand, but I personally have been forced to use (2) instead of (1), simply because (1) had multiple points of exit. I guess we both agree more than we disagree then :) Regards Senthil _____________________________ My Blog | My Articles | WinMacro
-
Probably i'm going to ask the stupid question of the thread, but... What has to do try/catch with goto? Can the goto statement catch an exception?
instead of:
for (int i=0;i<1000;i++)
{
if (i==500) goto end;
}
end;you would do this:
try
{
for (int i=0;i<1000;i++)
{
if (i==500) throw something;
}
}
catch (something e)
{
// el yay
} -
instead of:
for (int i=0;i<1000;i++)
{
if (i==500) goto end;
}
end;you would do this:
try
{
for (int i=0;i<1000;i++)
{
if (i==500) throw something;
}
}
catch (something e)
{
// el yay
} -
And why not... for(int i=0; i<1000; i++) { if(i==500) break; } -- modified at 7:50 Friday 5th May, 2006 I'd like to add that an exception should be used for exceptional cases that could happend in the execution, but not for flow control.
Gizzo wrote:
And why not
because you didn't ask about break and goto, you asked about try/catch and goto. Cleek | Image Toolkits | Thumbnail maker
-
Gizzo wrote:
And why not
because you didn't ask about break and goto, you asked about try/catch and goto. Cleek | Image Toolkits | Thumbnail maker
-
And why not... for(int i=0; i<1000; i++) { if(i==500) break; } -- modified at 7:50 Friday 5th May, 2006 I'd like to add that an exception should be used for exceptional cases that could happend in the execution, but not for flow control.
Because break; doesn't work if you have nested loops. In my opinion, using goto with a well-named label is better than setting a flag (often named "ok" or "abort") to leave nested loops.
-
ok, ok, you are right, but what i wanted to known is why people is comparing try/catch with goto, when they are different statements which should be used in diferent cases.
people have been taught that goto is evil. try/throw/catch can do what goto does (which is sometimes exactly what a function needs). and since try/throw/catch is not a goto, you aren't breaking the "NEVER YOU GOTOs" rule when you do it. Cleek | Image Toolkits | Thumbnail maker
-
Well, which piece of code do you think is more maintainable. 1.
HRESULT DoSomething(int param1, int param2)
{
if (IsInvalidParam1(param1))
return E_INVALIDPARAM1;
if (IsInvalidParam2(param2))
return E_INVALIDPARAM2;
if (param1 != param2)
return E_INVALIDARGS;
//Do Actual stuff
return S_OK;
}HRESULT DoSomething(int param1, int param2)
{
HRESULT ret = S_OK;
if (IsInvalidParam1(param1))
{
ret = E_INVALIDPARAM1;
}
else if (IsInvalidParam2(param2))
{
ret = E_INVALIDPARAM2;
}
else
{
if (param1 != param2)
ret = E_INVALIDARGS;
else
{
//Do Actual stuff
}
}
return ret;
}Regards Senthil _____________________________ My Blog | My Articles | WinMacro
-
Again, the answer is maintenance. I don't agree with rigid rules, but, if you should consider whether your code will be understood by people maintaining it. If you have numerous exit points in a function, then maybe the function is doing too much :).
Thomas George wrote:
If you have numerous exit points in a function, then maybe the function is doing too much
Possibly, but not always. Take for example a function that is verifying a user input against a set of rules. It makes sense to test one rule and return immediately if it fails, then test the next and return immediate if it fails etc.
Ryan
"Punctuality is only a virtue for those who aren't smart enough to think of good excuses for being late" John Nichol "Point Of Impact"
-
I've found this example (just look at SafeArrayGetElement)
STDMETHODIMP CEnumPoint::Next( ULONG celt, VARIANT FAR rgvar[], ULONG * pceltFetched) { /// ... omited for(i = 0; i < celt; ++i){ // Are we at the last element? if(m_iCurrent == m_celts){ hresult = S_FALSE; goto LDone; } ix = m_iCurrent++; // m_psa is a global variable that holds the safe array. hresult = SafeArrayGetElement(m_psa, &ix, &rgvar[i]); if(FAILED(hresult)) goto LError0; } hresult = NOERROR; LDone:; if (pceltFetched != NULL) *pceltFetched = i; return hresult; LError0:; for(i = 0; i < celt; ++i) VariantClear(&rgvar[i]); return hresult; }goto LError0; :omg: goto LDone; :wtf: i've been shaking some minutes but now i'm ok goto Work; :~PDSK docs are usually written in lowest-common-denominator C, since the Win32 API is a C API. So the code couldn't use RAII techniques to do automagic cleanup.
--Mike-- Visual C++ MVP :cool: LINKS~! Ericahist | PimpFish | CP SearchBar v3.0 | C++ Forum FAQ
-
PDSK docs are usually written in lowest-common-denominator C, since the Win32 API is a C API. So the code couldn't use RAII techniques to do automagic cleanup.
--Mike-- Visual C++ MVP :cool: LINKS~! Ericahist | PimpFish | CP SearchBar v3.0 | C++ Forum FAQ
Michael Dunn wrote:
PDSK docs are usually written in lowest-common-denominator C, since the Win32 API is a C API. So the code couldn't use RAII techniques to do automagic cleanup.
Yeah, but this code was in a member function - CEnumPoint::Next :-) Regards, Nish
Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
The Ultimate Grid - The #1 MFC grid out there!