6
\$\begingroup\$

After dipping my toes in PowerShell for over a year i think it is time for creating a real cmdlet. My first mission was creating a regular PS script to take in a .csv and create a wifi account using the Aerohive REST api.

Although it works, i find the script bloated and ugly, and want to swap large code block for proper cmdlets that can also be used by my colleagues.

This is the first step, authenticating against the api. At first i'm only trying to get the basic functions to work. In a later phase i'll add error handling and stuff.

For now i'm also using restsharp. If i ever complete this, i want to swap it for the builtin .net classes so there are no external dependencies.

The next cmdlets will be for creating and deleting wifi accounts. But at first i wanna know if i'm using the correct syntax, proper use of variables, and if everything is secure enough.

using System;
using System.Management.Automation;
using RestSharp;

namespace Aerohive
{
    [Cmdlet(VerbsCommon.New, "Token")]
    [OutputType(typeof(Token))]
    public class NewTokenCmdlet : PSCmdlet
    {
        [Parameter(
            Mandatory = true,
            ValueFromPipelineByPropertyName = true,
            HelpMessage = "The api key, as provided by the IDManager web interface.")]
        public string apiKey { get; set; }

        [Parameter(
            Mandatory = true,
            ValueFromPipelineByPropertyName = true,
            HelpMessage = "The username you use to login on the web interace of the IDManager")]
        public string username { get; set; }

        [Parameter(
            Mandatory = true,
            ValueFromPipelineByPropertyName = true,
            HelpMessage = "The password you use to login on the web interface of the IDManager")]
        public string password { get; set; }

        protected override void BeginProcessing()
        {
            base.BeginProcessing();
        }

        protected override void ProcessRecord()
        {
            WriteVerbose("Authenticating ...");

            var client = new RestClient();
            client.BaseUrl = new Uri("very.long.url/to/authenticate/against");

            var request = new RestRequest("authenticate", Method.POST);
            request.AddHeader("content-type", "application/json");
            request.AddJsonBody(new
            {
                token = apiKey,
                userName = username,
                password = password
            });

            WriteVerbose("response");
            IRestResponse<Token> response = client.Execute<Token>(request);
            WriteObject(response.Data);
        }

        protected override void EndProcessing()
        {
            base.EndProcessing();
        }
    }
    public class Token
    {
        public string token { get; set; }
        public int errorCode { get; set; }
        public string errorMessage { get; set; }
    }
}
\$\endgroup\$
2
\$\begingroup\$

You are writing a powershell commandlet... are you sure you want to hardcode the url?

client.BaseUrl = new Uri("very.long.url/to/authenticate/against");

I think it would be better to provide a parameter for this one too. At least make it optional with a default value if you don't want to type it everythime but hardcoding it - well, I woudn't.

It doesn't take long and you'll find another use for it but then you'll say if I only made a parameter for this :-)

\$\endgroup\$
-1
\$\begingroup\$

What is the point of doing this?

  protected override void EndProcessing()
    {
        base.EndProcessing();
    }

You're simply delegating to the superclass, so there's no sense in overriding the method. You can remove this.

\$\endgroup\$
15
  • 1
    \$\begingroup\$ I'm asking the same seeing you answer ;-P \$\endgroup\$
    – t3chb0t
    Aug 23 '16 at 7:49
  • \$\begingroup\$ @t3chb0t Ok, maybe i don't see something obvious. What is the sense of this piece of code ? What does it override ? \$\endgroup\$ Aug 23 '16 at 7:54
  • 2
    \$\begingroup\$ I don't know but your answer doesn't look like a review at all. It should be a comment. \$\endgroup\$
    – t3chb0t
    Aug 23 '16 at 7:56
  • 1
    \$\begingroup\$ This could make a good answer if you wouldn't phrase it as a question. You could state that it is senseless to have an override which is just calling the base version. \$\endgroup\$
    – Heslacher
    Aug 23 '16 at 8:39
  • 2
    \$\begingroup\$ @Disappointed The problem is that your answer wasn't clear, it could have been you genuinely wondering "What is this, what does it mean?". If the OP made the mistake in the first place, it's possible they don't understand that it's wrong, so it's always better to be clear about what you mean. \$\endgroup\$ Aug 23 '16 at 9:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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