#include <comdef.h>
#include <shlobj.h>
_COM_SMARTPTR_TYPEDEF(IFileOpenDialog, __uuidof(IFileOpenDialog));
_COM_SMARTPTR_TYPEDEF(IShellItem, __uuidof(IShellItem));
bool FileOpen(HWND window, LPCWSTR captionText, std::wstring &dirFileName)
{
IFileOpenDialogPtr fileOpenDlg(__uuidof(FileOpenDialog));
if(FAILED(fileOpenDlg->SetTitle(captionText)))
return false;
FILEOPENDIALOGOPTIONS fileOpenDlgOptions;
if(FAILED(fileOpenDlg->GetOptions(&fileOpenDlgOptions)))
return false;
fileOpenDlgOptions &= FOS_FILEMUSTEXIST;
fileOpenDlg->SetOptions(fileOpenDlgOptions);
HRESULT ret = fileOpenDlg->Show(window);
if(FAILED(ret))
return ret == HRESULT_FROM_WIN32(ERROR_CANCELLED);
IShellItemPtr resultShellItem;
if(FAILED(fileOpenDlg->GetResult(&resultShellItem)))
return false;
LPWSTR resultShellItemName = NULL;
if(FAILED(resultShellItem->GetDisplayName(SIGDN_FILESYSPATH, &resultShellItemName)))
return false;
dirFileName = resultShellItemName;
CoTaskMemFree(resultShellItemName);
return true;
}
Grade my simple C++ code [How would you make this better?]
Author
I've seen the gurus on this group come up with some insane stuff, so I have to ask, how could my simple code below be better?
[Edited by - Jacob Jingle on April 1, 2010 12:14:03 PM]
Can you give us some more information? I'm not at all qualified to review this kind of stuff, but maybe a summary of what this is trying to do, what kind of advice you're looking for or any concerns with how it's working would be helpful.
Author
Quote:Original post by darkcrobat
Can you give us some more information? I'm not at all qualified to review this kind of stuff, but maybe a summary of what this is trying to do, what kind of advice you're looking for or any concerns with how it's working would be helpful.
I'm just looking for alternative approaches, coding styles, things I should think about, etc to this kind of function. Anything anyone has to offer basically.
- Implementing it with a good widget library (QT 4) would increase the portability
- Avoiding hard-wired constant strings ("Open File") would help to localize your application
- What are other possible errors? Do you want to inform the user if the file exists but is inacessible (no reading permissions)?
One minor thing: Instead of this:
I would write:
if(FAILED(ret)) if(ret == HRESULT_FROM_WIN32(ERROR_CANCELLED)) return true; else return false;I would write:
if(FAILED(ret)) return ret == HRESULT_FROM_WIN32(ERROR_CANCELLED);if(FAILED(ret)) if(ret == HRESULT_FROM_WIN32(ERROR_CANCELLED)) return true; else return false;Use braces please.
if(FAILED(ret)){ if(ret == HRESULT_FROM_WIN32(ERROR_CANCELLED)) { return true; } else { return false; }}
Author
Quote:Original post by Decibit
- Implementing it with a good widget library (QT 4) would increase the portability
- Avoiding hard-wired constant strings ("Open File") would help to localize your application
- What are other possible errors? Do you want to inform the user if the file exists but is inacessible (no reading permissions)?
A) I really don't care about portability for the lil project I'm working on. QT would be overkill. BTW, doesn't WINE get rid of the need for this?
B) Yeah, I will change the code above to reflect this. In the end I will probably just throw it all into a class....Or not, I dunno.
C) Just let errors from control be all the warnings they get. Work anything else in later.
Thx for suggestions.
Author
Quote:Original post by Gage64
One minor thing: Instead of this:if(FAILED(ret)) if(ret == HRESULT_FROM_WIN32(ERROR_CANCELLED)) return true; else return false;
I would write:if(FAILED(ret)) return ret == HRESULT_FROM_WIN32(ERROR_CANCELLED);
Changed it. Thx for suggestion.
Quote:Original post by Jacob Jingle
BTW, doesn't WINE get rid of the need for this?
Only in a broad sense ("it works... somewhat"). First, not everything works with Wine. Second, GDI performance is abysmal; this is especially noticable with custom drawn GUIs. Third, properly ported software fits in with its environment, integrates with the system so to speak. Things like drag'n'drop between apps and desktop, filesystem conventions, etc... and many other things don't work or don't feel right with Wine.
You could probably do all your assigns in one bunch and then all your tests in another bunch. It's a style choice and not important, but it may improve readability.
Also, it needs comments! :)
Also, it needs comments! :)
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
