Is this an efficient way to parse function parameters?

Question

So I am new to C#, but one thing I already like coming from other higher level languages is the ability to do bitwise operations in (close to) C. I have a bunch of functions where some or all parameters are optional, and I like switches, so I built a function that converts boolean arrays to unsigned Shorts which allows me to basically Mux a boolean array to a single value for the switch:

namespace firstAsp.Helpers{
    public class argMux{                       
        public static ushort ba2ushort (bool[] parms){  
            //initialize position and output                
            ushort result = 0;
            int i = parms.Length-1;
            foreach (bool b in parms){
                if (b)//put a one in byte at position of b
                    //bitwise or with position
                    result |= (ushort)(1<<i);
                i--;
            }
            return result;               
        }
    }
}

Here is an example use case:

public IActionResult Cheese(string fname,string lname)
    {
        bool[] tf = {fname!=null,lname!=null};

        switch(argMux.ba2ushort(tf)){
         case 3:
            @ViewData["Data"]=$"Hello, {fname} {lname}";
            break;
         case 2:
            @ViewData["Data"]=$"Hello, {fname}";
            break;
         case 1:
            @ViewData["Data"]=$"Hello, Dr. {lname}";
            break;
         case 0:
            @ViewData["Data"]="Hello, Dr. CheeseBurger";
            break;
        }
        return View();
    }

My question is, Is this an efficient way to do this, or is there a way that is superior? I am aiming for simplicity of use which this definitely delivers for me, but I would also like it to be efficient code that is fast at runtime. Any pointers? Is this a stupid way to do this? Any and all feedback is welcome, you can even call me an idiot if you believe it, I'm not too sensitive. Thanks!

I have only been doing C# for about 3 days, so I wouldn't expect anything otherwise. Pls learn me your ways. — ThisGuyCantEven, Apr 27 '18 at 16:28

Eric Lippert · Accepted Answer · 2018-04-27T16:46:31.973

This is all bad.

The correct way to write your method is to use none of this:

public IActionResult Cheese(string firstName, string lastName)
{
    @ViewData["Data"]=$"Hello, {firstName ?? "Dr."} {lastName ?? "Cheeseburger"}";
    return View();
}

one thing I already like coming from other higher level languages is the ability to do bitwise operations in (close to) C.

If you are twiddling bits to solve a high level business problem, you are probably doing something wrong. Solve high-level business problems with high-level business code.

Also, if you are using unsigned types in C#, odds are good you are doing something wrong. Unsigned types are there for interoperability with unmanaged code. It is very rare to use a ushort or a uint or a ulong for anything else in C#. Quantities which are logically unsigned, like the length of an array, are always represented as signed quantities.

C# has many features which are designed to ensure that people who have COM libraries can continue to use their libraries, and so that people who need the raw, unchecked performance of pointer arithmetic can do so when it is reasonably safe. Don't mistake the existence of those low-level programming features as evidence that C# is typically used as a low-level programming language. Write your code so that it reads clearly as an implementation of a business workflow.

The business of your code is rendering a name as a string, so it should clearly read as rendering a name as a string. If I asked you to write down a name on a piece of paper, the first thing you'd do would not be to make a bit array to help you, so it shouldn't be here either.

Now, there might be some cases where this sort of thing is sensible, and in those cases you should use an enum rather than treating a short as a bit field:

[Flags]
enum Permissions 
{
  None = 0x00,
  Read = 0x01,
  Write = 0x02,
  ReadWrite = 0x03,
  Delete = 0x04,
  ReadDelete = 0x05,
  WriteDelete = 0x06,
  ReadWriteDelete = 0x07
}
...
static Permissions GetPermission(bool read, bool write, bool delete) {
  var p1 = read ? Permissions.Read : Permissions.None;
  var p2 = write ? Permissions.Write : Permissions.None;
  var p3 = delete ? Permissions.Delete : Permissions.None;
  return p1 | p2 | p3;
}

And now you have a convenient

switch(p)
{
  case Permissions.None: ...
  case Permissions.Read: ...
  case Permissions.Write: ...
  case Permissions.ReadWrite: ...

But notice here that we keep everything in the business domain. What are we doing? Checking permissions. So what does the code look like? Like it is checking permissions. Not twiddling a bunch of bits and then switching on an integer.

This is exactly what I needed, I had no ideathere wasan operator that accomplishes my exact goal (coalesce the nulls)! — ThisGuyCantEven, Apr 27 '18 at 16:32
@ThisGuyCantEven: Quite fine. Also, get out of the habit of `fname`, `ba2ushort ` and so on. You will not die younger if you type `firstName`, `PackBooleans` and so on. **Use full words**. And **methods should be verbs that describe their action**. — Eric Lippert, Apr 27 '18 at 16:36
Noted on the naming convention advice. Yeah.. the only reason I was using unsigned was to avoid doing extra math for the sign bit. But none of that matters now. Thanks! — ThisGuyCantEven, Apr 27 '18 at 16:40
@ThisGuyCantEven: Can you describe what problem you believed you might have with the sign bit? C# will give you a warning in most cases where you are going to make a mistake with a sign bit. Also, I've added some thoughts on the correct way to do your switch logic in C#. — Eric Lippert, Apr 27 '18 at 16:47
well I guess I could have just started at a higher index like `params.length` instead of `params.length-1` to avoid any issue with the sign bit. but the way I implemented it was to push to the actual position in the array which would have meant possibly putting a 1 in the sign bit, and definitely would have resulted in division by 2. Though perhaps my assumption that the first bit of a signed int is the sign and putting a 1 in it would make the value negative is also wrong. — ThisGuyCantEven, Apr 27 '18 at 17:43
Am I also mistaken in thinking that pushing the `bool[]` as binary to the front of a signed int the way I did would cause a right shift of 1 due to the sign bit being on the front? — ThisGuyCantEven, Apr 27 '18 at 17:50
@ThisGuyCantEven: It's a bad idea to think of the high bit of a signed int as "the sign bit". Not because it is wrong -- if the number is negative then that bit will be on -- but because integers are not like doubles and decimals. Doubles and decimals really do have a sign bit; if you flip that bit, then `x` becomes `-x`. But integers do not have the property that flipping the sign bit changes the sign of the value. — Eric Lippert, Apr 27 '18 at 19:06
@ThisGuyCantEven: Regardless, though your code is bad C# style, and I think it weird that `parms[n]` does not correspond to bit n, there's no problem in your code associated with the high bit of a ushort. — Eric Lippert, Apr 27 '18 at 19:10

Is this an efficient way to parse function parameters?

1 Answers1