Einen Link auflösen

3447
Quelle (EN)
Billy ONeal

Wie kann ich das aufräumen?

std::wstring LinkResolve::ResolveLink( const std::wstring& source ) const
{
    HRESULT errorCheck;
 wchar_t linkTarget[MAX_PATH];
 wchar_t expandedTarget[MAX_PATH];
 wchar_t arguments[INFOTIPSIZE];
    ATL::CComPtr<IPersistFile> ipf;
    errorCheck = ipf.CoCreateInstance(CLSID_ShellLink, 0, CLSCTX_INPROC_SERVER);
    if (!SUCCEEDED(errorCheck))
    {
        throw _com_error(errorCheck);
    }
    errorCheck = ipf->Load(source.c_str(), 0);
    ATL::CComPtr<IShellLink> shellLink;
    errorCheck = ipf->QueryInterface(&shellLink);
    if (!SUCCEEDED(errorCheck))
    {
        throw _com_error(errorCheck);
    }
    errorCheck = shellLink->Resolve(0, SLR_NO_UI);
    if (!SUCCEEDED(errorCheck))
    {
        throw _com_error(errorCheck);
    }
    errorCheck = shellLink->GetPath(linkTarget, MAX_PATH, 0, SLGP_RAWPATH);
    if (!SUCCEEDED(errorCheck))
    {
        throw _com_error(errorCheck);
    }
    ExpandEnvironmentStringsW(linkTarget, expandedTarget, MAX_PATH);
    errorCheck = shellLink->GetArguments(arguments, INFOTIPSIZE);
    if (SUCCEEDED(errorCheck))
    {
        return std::wstring(expandedTarget) + L" " + arguments;
    }
    else
    {
        return expandedTarget;
    }
}
Antworten
36
Ich denke dabei an Haskells vielleicht Monade, aber ich glaube nicht, dass das Ihnen helfen wird. Trotzdem würde das völlig funktionieren. Theo Belaire vor 10 Jahren 1
@Tyr: Or C++14's optional. :) Billy ONeal vor 7 Jahren 0

6 Antworten auf die Frage

56
CB Bailey

Persönlich würde ich eine einfache Funktion schreiben:

void ThrowOnFail( HRESULT hrcode )
{
    if (FAILED(hrcode))
        throw _com_error(hrcode);
}

Dann werden die Funktionsaufrufe:

ThrowOnFail( ipf.CoCreateInstance(CLSID_ShellLink, 0, CLSCTX_INPROC_SERVER) );
ThrowOnFail( ipf->Load(source.c_str(), 0) );
ATL::CComPtr<IShellLink> shellLink;
ThrowOnFail( ipf->QueryInterface(&shellLink) );
ThrowOnFail( shellLink->Resolve(0, SLR_NO_UI) );
ThrowOnFail( shellLink->GetPath(linkTarget, MAX_PATH, 0, SLGP_RAWPATH) );

Sie haben übrigens einen Check für errorCheckdanach verpasst Load. Dies wird mit einer Check-Funktion leichter zu erkennen.

Wäre es möglich oder sinnvoll, den gesamten Codeblock in einem ThrowOnFail (...) zusammenzuführen? einschließlich des CComPtr? Das würde auch die Duplizierung von Code reduzieren. Nicht sehr vertraut mit C ++ und CComPtr. WernerCD vor 10 Jahren 0
@WernerCD: Nein, das wäre nicht möglich. (Außerdem hätten Sie bei all diesen Methodenaufrufen eine Zeile mit 500 Zeichen!) Billy ONeal vor 10 Jahren 0
Ähm ... Sie wissen, das ist wirklich der Grund, warum ich um 3 Uhr morgens keinen Code schreiben sollte: sigh :. Billy ONeal vor 10 Jahren 1
Frage: Identifiziert der hieraus zurückgegebene Fehler eindeutig, woher der Fehler kam? Hack Saw vor 10 Jahren 0
18
Arlaharen

Wenn ich finde, dass mein Selbst immer wieder dasselbe schreibt, stelle ich es normalerweise irgendwo in eine Funktion. Auch wenn diese Funktion in Ihrem Fall so einfach ist:

void check(HRESULT result) {
    if (FAILED(result)) {
        throw _com_error(result);
    }
}

Ich denke, der Code sieht ziemlich einfach aus, wenn Sie Ihren Fehlerprüfcode wiederverwenden. Ich kenne die von Ihnen verwendete API nicht. Daher kann ich nicht kommentieren, ob es eine andere Methode gibt, die zu sauberem Code führt.

Darf ich Sie bitten, "! SUCCEED" durch "FAILED" zu ersetzen? Letzteres wird genau so wie "! SUCCEEDED" ausgewertet. sharptooth vor 10 Jahren 0
6
DeadMG

Zumindest bei der Verwendung von DirectX verwende ich ein Makro.

#define D3DCALL(a) { auto __ = a; if (FAILED(__)) DXTrace(__FILE__, __LINE__, __, WIDEN(#a), TRUE); }

Sie könnten schicker werden und einen Typ mit einem Operator = (HRESULT) verwenden, um die Prüfung durchzuführen.

Warum ein überall reservierter Name für die lokale Variable? Ich würde es _local_D3DCALL nennen (was im Root-Namespace-Bereich reserviert ist, aber nicht anderswo) oder ähnlich. Dieses Makro ist ein guter Kandidat für den Aufruf einer (Inline-) Funktion, wobei (a), \ _ \ _ FILE \ _ \ _, \ _ \ _ LINE \ _ \ _ und #a übergeben werden. Fred Nurk vor 10 Jahren 1
Keine Ahnung, es ist lange her, seit ich es geschrieben habe, es ist nur eine Probe. DeadMG vor 10 Jahren 0
Einige Makros nur für Sie! -> http://codereview.stackexchange.com/questions/281/the-same-if-block-over-and-over-adain-oh-my-part-2-now-the-macro-monster-got: ) Billy ONeal vor 10 Jahren 0
3
MarkS

Ich erinnere mich, dass ich so etwas schon einmal gesehen habe:

class XHR
{
public:
   XHR() {};
   ~XHR() {};

   operator HRESULT() { return hr };

   HRESULT& operator =(const HRESULT& rhs)
   {
       hr = rhs;
       if (FAILED(hr)
          throw _com_error(hr);
   }

private:
   HRESULT hr;
}

Dann kannst du schreiben:

std::wstring LinkResolve::ResolveLink( const std::wstring& source ) const
{
    XHR errorCheck;   // instead of HRESULT errorCheck

    wchar_t linkTarget[MAX_PATH];
    wchar_t expandedTarget[MAX_PATH];
    wchar_t arguments[INFOTIPSIZE];
    ATL::CComPtr<IPersistFile> ipf;


    errorCheck = ipf.CoCreateInstance(CLSID_ShellLink, 0, CLSCTX_INPROC_SERVER);
    errorCheck = ipf->Load(source.c_str(), 0);

    ATL::CComPtr<IShellLink> shellLink;

    errorCheck = ipf->QueryInterface(&shellLink);
    errorCheck = shellLink->Resolve(0, SLR_NO_UI);
    errorCheck = shellLink->GetPath(linkTarget, MAX_PATH, 0, SLGP_RAWPATH);

    ExpandEnvironmentStringsW(linkTarget, expandedTarget, MAX_PATH);

    try {
       errorCheck = shellLink->GetArguments(arguments, INFOTIPSIZE);
       return std::wstring(expandedTarget) + L" " + arguments;
    }
    catch (const XHR&)
    {
        return expandedTarget;
    }
}

Jedes Mal, wenn ein HRESULT einen Fehler anzeigt, wird es automatisch in einen _com_error für Sie konvertiert und geworfen.

Das ist interessant (deshalb werde ich Sie +1), aber wenn Sie die Art von `errorCheck` ignorieren würden, würde es so aussehen, als würden Sie Fehler ignorieren. Ich denke, ich würde einen Ansatz bevorzugen, der deutlicher erscheint, wenn Sie Aussagen betrachten und nur die Typen betrachten. asveikau vor 9 Jahren 1
1
Dani

Fehlerprüfungen sind nicht alle gleich. Manchmal handeln Sie nach speziellen HR-Ergebnissen. manchmal gibt es eine ELSE. Manchmal möchten Sie den Fehler protokollieren, manchmal nicht.

Obwohl es unwahrscheinlich ist, dass es in der com / atl-Welt relevant ist, hat das Aufrufen einer Funktion ihre Leistungskosten.

Ich ziehe es vor, if nach dem Aufruf zu verwenden, anstatt eine Funktion aufzurufen. Wie viel sparen Sie? 10 Zeichen eingeben?

Das Beispiel in der Frage ist sehr klar, dass es immer und immer wieder derselbe Code ist. Wenn es nicht funktionalisiert wird, entsteht nur ein Wartungskopfschmerz. Sollte es erforderlich sein, einen bestimmten Fall anders zu behandeln, schreiben Sie dafür spezifischen Code. Aber das als Entschuldigung zu verwenden, um die allgemeine Funktion überhaupt nicht zu schreiben, ist ineffizient und die IMO ist schlecht beraten. LRE vor 10 Jahren 6
Wenn dies C ++ ist, können Sie die Methode immer inline setzen oder ein Makro mit derselben Funktionalität verwenden Pedro Loureiro vor 10 Jahren 0
Verwenden Sie kein Makro. Sie verlieren alle Typensicherheit. Das ist der Grund für Inline-Funktionen / -Methoden. Mark Loeser vor 10 Jahren 0
@ MarkLoesser - Es gibt Zeiten, in denen Makros das richtige Werkzeug für den Job sind. Dies ist eine dieser Zeiten. Die Erweiterungen __FILE__ und __LINE__ sind sehr hilfreich bei der Problembehandlung und gehen verloren, wenn Sie eine Inline-Funktion oder -Methode verwenden. Jere.Jones vor 10 Jahren 2
1
Autodidact

Ich denke, Sie können dasselbe tun, indem Sie die Compiler-Com-Unterstützung verwenden. Hier ist ein Beispiel.

#import "CLSID:lnkfile" //use the clsid of the ShellLink class.

IPersistFilePtr ptr = IPersistFilePtr.CreateInstance(...);

_com_ptr_t::CreateInstance()wird eine Ausnahme auslösen (vom Typ, _com_errorwenn der Aufruf fehlschlägt)

Alle anderen ifs in Ihrem Code können mithilfe der von #import generierten intelligenten Zeiger ersetzt werden. Ich weiß, dass ich ein bisschen klein auf Details bin, aber es ist lange her, seit ich COM berührt habe.

Nicht zutreffend gemäß [this] (http://msdn.microsoft.com/de-de/library/k2cy7zfz.aspx); `_com_ptr_t :: CreateInstance ()` scheint eine `HRESULT` zurückzugeben und hat eine` throw () `Spezifikation. Billy ONeal vor 10 Jahren 0

Verwandte Probleme