Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a wrapper which interacts with a third party component. In Each function i am performing the operation if it the component is initialized.

I have other function to check the errorCode and component initialization check.

Is this right way to do this? I am not happy with the readability of this code. Help me improving this code.

Thanks in advance.

Void Operation1 (parameter someInput)
{   
    int errorCode= 0;
    if (Initialized)
    {  
        errorCode = ThirdPartyComponent.Operation1(someInput);
    }

     CheckErrorCodeAndShowError ("Operation1 with someInput failed", returnValue);    
}

Void Operation2 (parameter someInput)
{   
    int errorCode= 0;
    if (Initialized)
    {  
        errorCode = ThirdPartyComponent.Operation2(someInput);
    }

    CheckErrorCodeAndShowError ("Operation2 with someInput failed", returnValue);
}

Void CheckErrorCodeAndShowError (String errorMessage, int errorCode)
{   
   if (!Initialized)
   {
       //Show not initialized error
   }

   if (errorCode == -1)
   {
      //Show Operation failed error (errorMessage)
   }
   else if (errorCode == 0)
   {
      //Show operation success message etc
   }
   //other error code check
}
share|improve this question
    
Does the third party have some sort of mapping of error codes to strings? Most of the third party libraries I have used have had something like that. Either a function I could call with the error code that would return a string or a file that contained a definition of the values with some comments of what they mean. –  pstrjds Jan 3 '13 at 7:17
    
No, it does not have any mapping. –  NatGio Jan 3 '13 at 7:30
    
Does it always return -1 when there is an error? Does it not have any way to get some extended information. Like a Windows "GetLastError" type of call? –  pstrjds Jan 3 '13 at 7:44
    
It has many error codes. For simplicity i showed only -1. But no other extended information is available from that component. –  NatGio Jan 3 '13 at 8:21

1 Answer 1

up vote 1 down vote accepted

Okay, well here is what I would probably do in this situation. If the error codes you are getting are consistent this will make this type of solution even better (consistent as in "FileNotFound" error code is always -42 or something like that).

I would add a private method that throws an "Uninitialized" exception if the object has not been initialized. There is no need to be checking that twice for each function call.

private void ValidateInitialized()
{
    throw new LibraryUninitializedException(); // Just create this exception class
}

Then I would create a helper method that takes a Func<int> that you can call from each of the wrapper methods. The helper method will handle the error code stuff for you. It would look something like this (note: I have not compiled this, so I may have some minor syntax errors).

private void DoThirdPartyOperation(Func<int> operation, string message = "")
{
     ValidateInitialized();
     switch(operation())
     {
         case 0:
             // No error - do nothing
             break;
         case someErrorCode:
             throw new SomeErrorException(message);
             break;
         case someOtherErrorCode:
             throw new SomeOtherErrorException(message);
             break;

         default:
             // Choose either to throw some sort of general exception and include
             // the error code in it OR throw an UnrecognizedErrorException
     }
}

public void Operation1(someParameter)
{
    DoThirdPartyOperation(() => { ThirdParty.Operation1(someParameter); });
}

public void Operation2(someParameter)
{
    DoThirdPartyOperation(() => { ThirdParty.Operation2(someParameter); },
        "Failed performing Operation2");
}
share|improve this answer
    
Thanks pstrjds. I agree with your first part (ValidateInitialized), but the second is is not ok, due to incomplete information which i provided. Because, each operation may need different type of input and number of inputs. –  NatGio Jan 3 '13 at 8:53
    
Okay. I kind of thought maybe that was the case. What is the third party library you are using? Can you provide additional details? –  pstrjds Jan 3 '13 at 8:56
    
It is proprietary component, internal to our organization (at the same time we don't have control to change now:( ). –  NatGio Jan 3 '13 at 9:09
    
Even though it solved only half of my problem, i am marking this as answer because the problem is not with the answer it is with the question. I haven't explained him fully at first. –  NatGio Jan 3 '13 at 12:26

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.