Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
User name:
Password:
Save Password
Forgot your password?

 All Forums
 Visual Assist
 Technical Support
 No good result of Code inspection
 New Topic  Reply to Topic
 Printer Friendly
Author Previous Topic Topic Next Topic  

xMRi
Tomato Guru

Germany
315 Posts

Posted - Dec 14 2022 :  08:10:11 AM  Show Profile  Reply with Quote

static DWORD GetScale(LPCTSTR &lpszText, bool &fError)
{
	fError = false;
	// Wert folgt?
	if (*lpszText!=_T('['))	
		return CResizeWndHandler::SCALE_1_1;

	// Wert laden	
	LONG lValue1 = _tcstol(lpszText+1,const_cast<LPTSTR*>(&lpszText),10);	
	if (*lpszText==_T(']'))
	{
		++lpszText;
		return CResizeWndHandler::SCALE(1,lValue1);
	}
	else if (*lpszText==_T('/'))
	{
		++lpszText;
		LONG lValue2 = _tcstol(lpszText,const_cast<LPTSTR*>(&lpszText),10);
		if (*lpszText==_T(']'))
			++lpszText;
		else
			fError = true;
		return CResizeWndHandler::SCALE(lValue1,lValue2);
	}
	else
	{
		fError = true;
		return CResizeWndHandler::SCALE_1_1;
	}	
}


This line gets an "Unnecessary use of else after early exit.

else if (*lpszText==_T('/'))

The code will be changed to:
static DWORD GetScale(LPCTSTR &lpszText, bool &fError)
{
	// Clear error Flag
	fError = false;
	// Wert folgt?
	if (*lpszText!=_T('['))	
		return CResizeWndHandler::SCALE_1_1;

	// Wert laden	
	LONG lValue1 = _tcstol(lpszText+1,const_cast<LPTSTR*>(&lpszText),10);	
	if (*lpszText==_T(']'))
	{
		// Nur ein Wert
		++lpszText;
		return CResizeWndHandler::SCALE(1,lValue1);
	}
	if (*lpszText==_T('/'))
	{
		// zweiten Wert laden
		++lpszText;
		LONG lValue2 = _tcstol(lpszText,const_cast<LPTSTR*>(&lpszText),10);
		if (*lpszText==_T(']'))
			++lpszText;
		else
			// Syntax Fehler, ans Ende springen
			fError = true;
		return CResizeWndHandler::SCALE(lValue1,lValue2);
	}
	else
	{
		// Syntax Fehler, ans Ende springen
		fError = true;
		return CResizeWndHandler::SCALE_1_1;
	}	
}


For me this code change will alter the code into an unreadable version.
The (original) code with an else is more readable.
I think a switch case here would do a better job.

Martin Richter [rMVP] WWJD http://blog.m-ri.de
"A well-written program is its own heaven; a poorly written
program is its own hell!" The Tao of Programming

feline
Whole Tomato Software

United Kingdom
18751 Posts

Posted - Dec 14 2022 :  08:51:13 AM  Show Profile  Reply with Quote
You get the same suggestion from clang-tidy on the command line, which makes sense, since VA is using clang-tidy for Code Inspection. Personally I tend to agree, it "feels" wrong to remove the else there, but I can see an argument that it is "better". I think this is a case where the rule and our expectations can be going in different directions.

If you find this code inspection more irritating than helpful you can disable this specific check in VA Options.

zen is the art of being at one with the two'ness
Go to Top of Page
  Previous Topic Topic Next Topic  
 New Topic  Reply to Topic
 Printer Friendly
Jump To:
© 2023 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000