Seeking constructive criticism on first Arduino library

Hello, all.

I am new to the world of Arduino. I picked one up this weekend and have been having fun playing with it. I decided to create my own library for the MAX517 8-bit DAC.

Since I have never done this before, I was hoping to get some constructive criticism on the libraries' implementation.

It is hosted on GitHub: GitHub - JordanGoulder/Max517Dac: MAX517 8-bit DAC Library for Arduino

Thanks for the help!

Hi,

Your code looks generally good to me. You have declared a class so that you can support multiple instances with different addresses (if that is possible for that device), which is good. I have just a few minor comments:

  1. The static storage class on the declarations of constants CMD_RESET etc. is redundant. In C++ (although not in C), non-local constant declarations have file scope unless they are declared extern.

  2. You are using type int everywhere, however type uint8_t or byte is more appropriate and more efficient for parameters and constants that represent 8-bit values to be transmitted or received. This applies to constants like CMD_RESET and parameters like the ones to mSendCommand.

  3. You don't actually need to indicate an empty parameter list using (void), that is how you do it in C. In C++ it is sufficient and more usual to use just () instead. But it really doesn't matter.

  4. You're making the assumption that either the user will never want to call setOutput with a bit pattern of all 1s, or that if he does, he will pass it as value 255 not -1. Better I think to make the parameter uint8_t or byte so that this issue doesn't arise.

Thanks for the very helpful advice dc42. I have made changes to fix this issues that you mentioned.