0

I have declared a string extension method to decode a Base64 string something like :

        public static string DecodeFromBase64String(this string base64String)
        {
            return Encoding.UTF8.GetString(Convert.FromBase64String(base64String));
        }

Now there is an async function which uploads a file and returns me a base64 encoded value of its id. So is it fine if I simply pass this function with await in parameter, something like:

// Approach 1
var fileId = StringExtensions.DecodeFromBase64String( await uplodFile(FilePath) );

Or should I just do a two step process, first get base64 id, and then decode it in next step. This requires me to either declare two variables with meaningful names, or just reuse same variable

// Approach 2
var base64fileId = await uploadFile(FilePath); 
var fileId = StringExtensions.DecodeFromBase64String(base64fileId);

// Approach 3
var fileId = await uploadFile( Filepath); // at this step file Id doesn't make much sense, but reduces need of extra variable
fileId = StringExtensions.DecodeFromBase64String(fileId);

Which approach do you think is best considering design principles. I am still learning C#, so please excuse any of my mistakes.

  • My preference is Approach 2: a) splitting it into two expressions makes it easier to single-step through the debugger, if you ever need to troubleshoot the code; b) C# strings are [immutable](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/strings/) - "fileId" and "base64fileId" will always be separate and distinct CLR objects, so you might as well give them distinct names. – paulsm4 Mar 01 '21 at 18:03

1 Answers1

2

You're asking if "it's fine" to call dependent functions inline, or if you need temporary variables. The answer is of course it's fine.

Whether or not one way is more readable than the other might be debatable, but in your specific example I'd say the inline version is more readable since there's less noise and temporary variables to parse at a glance (assuming you get read of that noisy Encoding. namespace and use the => form of the function call for even less noise). Consider this:

    public static string DecodeFromBase64String(this string base64String) =>
        UTF8.GetString(Convert.FromBase64String(base64String));

One note about this though:

but reduces need of extra variable

That is a terrible argument for anything. The IL generated will use the stack for that function call, and the generated native code will use registers as needed. You're only making your code harder to parse for no reason.

Blindy
  • 65,249
  • 10
  • 91
  • 131
  • Also one more thing about actual performance, the way you decode that string is very inefficient. Not only in terms of processing power, but more importantly in terms of allocations and GC pressure, you're allocating three difference objects (the base64 string, the array and the resulting string, plus whatever else those functions allocate). You can easily bring it down to two allocations by hand-writing the base64 decoding into a `StringBuilder`, or even better a `Span`. Though I cannot possibly imagine why you'd base64 encode an actual string, just send it as it is. – Blindy Mar 01 '21 at 18:09
  • Actually the uploadFile function is a third party function, so decoding it becomes necessary. Also that decoding function has more checks in it, since it's a general purpose string extension method written,so for clarity of question I simplified the function. I just had doubt that passing await inline is fine or not. Thanks – Divyanshu Kumar Mar 02 '21 at 01:23