Here's some CeeStyleCeePlusPlus code from everyone's favorite software shop:

 BOOL C''''''reateURLShortcut(LPWSTR pszURL, LPWSTR pszShortcutFile) {
   WCHAR pszShortcutContents[1024];

   HANDLE hf = CreateFileW(pszShortcutFile, GENERIC_READ | GENERIC_WRITE, (DWORD) 0, NULL, CREATE_ALWAYS,
                            FILE_ATTRIBUTE_NORMAL, (HANDLE) NULL); 
   if(hf == INVALID_HANDLE_VALUE) return FALSE;
   int iCharCount = swprintf(pszShortcutContents, SHORTCUT_TEMPLATE, pszURL) + 1;
   DWORD dwWritten = 0;
   if(!W''''''riteFile(hf, pszShortcutContents, sizeof(WCHAR)*iCharCount, &dwWritten, NULL)) return FALSE;
   C''''''loseHandle(hf);

   return TRUE;
 }
That one little sample contains...

* a potential array bounds overflow
	* No it doesn't. I believe that swprintf is a typo for wsprintf, which has a hardcoded maximum buffer size of 1024 elements.
		* I've never heard of such a limit. But regardless, there is an swprintf(), as verified by a few seconds of Googling. It expects SHORTCUT_TEMPLATE to be a length parameter, though, which in turn suggests that our buffer size should be consistent (e.g. SHORTCUT_TEMPLATE+1). Also, we're screwed if the 'pszURL' happens to contain any % signs.
* a potential leak of a file handle
* HungarianNotation
* the usual discarded error values
* gratuitous typecasts ''using C-style casts, at that''
** Where are all the typecasts? I see only two. Two hardly counts as gratuitous. Moreover, all casts are for numeric constants, where the compiler won't likely know how to properly cast them anyway ''without'' the casts. I think you're just a wee bit overreacting to this code.
* a #define used as a constant, SHORTCUT_TEMPLATE, defined far from its only use
** The Win32 API is coded for C, not C++, so it makes sense that they utilize C-style API usage patterns. This is one of them. (SamuelFalvo)

Oh, and the program around it used VoidMain. But at least it doesn't lead us astray by writing a small wide text file robustly using the non-MS-controlled std::wofstream!!

----

CharlesPetzold celebrates the 20th anniversary of his cash cow, MicrosoftWindows, by ranting on this subject here:

http://charlespetzold.com/etc/DoesVisualStudioRotTheMind.html

----
Things could have looked like this or similar:

 bool CreateURLShortcut(const std::wstring& url, const std::wstring& shortcutFile)
 {
  using namespace std;
  wofstream of( shortcutFile, ios::out | ios::trunc );
  if( of )
  {
    return ( of << url );
  }
  return false;
 }
I suppose this body would work too, but little terse for my liking:
 ...
 {
  using namespace std;
  wofstream of( shortcutFile, ios::out | ios::trunc );
  return (of << url);
 }
''Not too terse for me. My usual set of ReFactoring''''''s would converge on that anyway (first by introducing a guard clause, then by merging the two conditionals with &&, then by observing that the test is redundant). But anyway, I read the last statement as "return whether we could send url to of", so the only part of it that's too terse for me is the stream name. I might even do it all in one line with a temporary:''

 return wofstream(shortcutFilename, ios::out | ios::trunc) << url;
''"return whether we could send url to a wofstream constructed using shortcutFilename with out and trunc flags". Makes perfect sense to me. -- KarlKnechtel''
----
CategoryCoding