开发者

Help me clean up this crazy lambda with the out keyword

My code looks ugly, and I know there's got to be a better way of doing what I'm doing:

private delegate string doStuff(
    PasswordEncrypter encrypter, RSAPublicKey publicKey,
    string privateKey, out string salt
);

private bool tryEncryptPassword(
    doStuff encryptPassword,
    out string errorMessage
)
{
    ...get some variables...
    string encryptedPassword = encryptPassword(encrypter, publicKey,
        privateKey, out salt);
    ...
}

This stuff so far doesn't bother me. It's how I'm calling tryEncryptPassword that looks so ugly, and has duplication because I call it from two methods:

public bool method1(out string errorMessage)
{
    string rawPassword = "foo";
    return tryEncryptPassword(
        (PasswordEncrypter encrypter, RSA开发者_开发百科PublicKey publicKey,
            string privateKey, out string salt) =>
            encrypter.EncryptPasswordAndDoStuff( // Overload 1
                rawPassword, publicKey, privateKey, out salt
            ),
        out errorMessage
    );
}

public bool method2(SecureString unencryptedPassword,
    out string errorMessage)
{
    return tryEncryptPassword(
       (PasswordEncrypter encrypter, RSAPublicKey publicKey,
           string privateKey, out string salt) =>
           encrypter.EncryptPasswordAndDoStuff( // Overload 2
               unencryptedPassword, publicKey, privateKey, out salt
           ),
       out errorMessage
   );
}

Two parts to the ugliness:

  • I have to explicitly list all the parameter types in the lambda expression because of the single out parameter.
  • The two overloads of EncryptPasswordAndDoStuff take all the same parameters except for the first parameter, which can either be a string or a SecureString. So method1 and method2 are pretty much identical, they just call different overloads of EncryptPasswordAndDoStuff.

Any suggestions?

Edit (solution): I ended up using Jeff's suggestion and altering the overloads of EncryptPasswordAndDoStuff to return an instance of EncryptionResult. Then I didn't need an explicitly defined delegate, and I used the following code:

private bool tryEncryptPassword(KeysAndEncrypter keys,
    Func<EncryptionResult> encryptPassword,
    out string errorMessage
) { ... }

private class KeysAndEncrypter
{
    public RSAPublicKey PublicKey { get; set; }
    public string PrivateKey { get; set; }
    public PasswordEncrypter Encrypter { get; set; }
}

And here was the contents of method1, with method2 being very similar:

string rawPassword = "foo";
KeysAndEncrypter keys = getEncryptionKeys();
return tryEncryptPassword(keys, () =>
    keys.Encrypter.EncryptPasswordAndDoStuff(
        rawPassword, keys.PublicKey, keys.PrivateKey
    ),
    out errorMessage
);


You could introduce a new type to represent the delegate's return value:

 public class EncryptionResult {
     public string EncryptedValue { get; set; }
     public string Salt { get; set; }
 }

... and change the delegate to something like this:

 private delegate EncryptionResult doStuff(
     PasswordEncrypter encrypter, 
     RSAPublicKey publicKey,
     string privateKey);


Why bother with the crazy lambda at all?

public bool Method1(out string errorMessage)
{
    return TryEncryptPassword("foo", out errorMessage);
}

public bool Method2(SecureString password, out string errorMessage)
{
    return TryEncryptPassword(password, out errorMessage);
}

private bool TryEncryptPassword(string password, out string errorMessage)
{
    GetSomeVariables();
    string encryptedPassword = encrypter.EncryptPasswordAndDoStuff(
           password, publicKey, privateKey, out salt);
    DoMoreStuff();
    // ...
}

private bool TryEncryptPassword(SecureString password, out string errorMessage)
{
    GetSomeVariables();
    string encryptedPassword = encrypter.EncryptPasswordAndDoStuff(
           password, publicKey, privateKey, out salt);
    DoMoreStuff();
    // ...
}

private void GetSomeVariables() { /* ...get some variables... */ }

private void DoMoreStuff() { /* ... */ }


Jeff's suggestion is reasonable. You could just use a Tuple<string, string> too though if you don't want to define a new type each time you need an out parameter.

Another suggestion would be to pass in another delegate to retrieve the error rather then using an out param.

private bool tryEncryptPassword(
     doStuff encryptPassword,
     Action<string> setError
)

public bool method1(Action<string> setError) {
    ...
    tryEncryptPassword(..., ..., ..., setError);
}

Before calling method1, you can assign your setError to a local variable.

string error;
Action<string> setError = str => error = str;
method1(setError);        


Also, if you don't want to create a custom type, you could return a tuple here.

private delegate Tuple<string, string> doStuff(
   PasswordEncrypter encrypter, 
   RSAPublicKey publicKey,
   string privateKey);

within doStuff, just create and return a tuple:

Tuple<string, string> result = new Tuple<string, string>(EncryptedValue, Salt);
return result;
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜