Calling Command

I know I am supposed to be using (and loving) power shell, but I ran across a problem this weekend and the good-old command window worked fine.  I was building a one-click application that moved data from one location to another and then manipulating the data.  As part of the workflow, I created a DTS/SSIS package.  To execute this package, I used the following code to shell out to the command prompt and fire up the package:

            String sourceConnectionString = CreateSourceConnectionString();
            String targetConnectionString = CreateTargetConnectionString();

            Process process = new Process();
            process.StartInfo.FileName = "cmd";
            StringBuilder stringBuilder = new StringBuilder();
            stringBuilder.AppendFormat(@"/k");
            stringBuilder.AppendFormat(@"dtexec");
            stringBuilder.AppendFormat(@" /F");
            stringBuilder.AppendFormat(@" TransferAllTables.dtsx");

            stringBuilder.AppendFormat(" /Conn {0};\"{1}\"", "SourceConnectionOLEDB", sourceConnectionString);
            stringBuilder.AppendFormat(" /Conn {0};\"{1}\"", "DestinationConnectionOLEDB", targetConnectionString);
            process.StartInfo.Arguments = stringBuilder.ToString();

            process.Start();

I also want to thank my friend Ian (who doesn’t have a blog yet so I can’t point you to him) for mentioning  the StringBuilder.AppendFormat() function.  Append() + String.Format() in 1 place.  Nice! Thanks  Ian!

Advertisements

NOAA and how not to do a web service

I came across the NOAA API when I was looking at various providers of weather data via the programmable web.  Thinking that the government might be a great place to get (free) data, I dove into their API.  I am glad I didn’t go head-first.  The API is, well, wretched.  In fact, it probably is the worst public API I have come across in my limited travels.

Why is so bad?

1) Ambiguous Website.  Their use of jargon is over-whelming.  To understand the API, you need to learn about the NDFD .  What is that?  What about current weather?  Nope, I need to know about the National Digital Forecast Database.  How about the NCDC?  What is DWML? On to issue #2.

2) They invented their own version of SOAP: Digital Weather Markup Language.  Enough said.

3)  The web site is rife with links that show graphics that no one can use.  The API help is in clear language?  No where to be seen.

4) Hooking up to their WSDL is not much better.  I made a connection and this is what I got back:

  image

Got that?  You need to create an instance of ndfdXMLPortTypeClient.  Say that 3 times fast.  How about a weather class?  A forecast class?  Nope, this API assumes that other developers give a hoot about their internal implementation (and no one does). 

5) I tried a simple call to the web service just to see what it sent back:

public WeatherReading GetReading(string zipCode)
{
    ndfdXMLPortTypeClient client = new ndfdXMLPortTypeClient();
    String output = client.LatLonListZipCode("27519");
    return null;
}

And what did I get?

image

Got that – a non-standard encoding.  PTF (Palm to face…)

So I am giving up on the government and trying some of the other providers.

Using the Twitter API

As part of my alerting project, I wanted to implement a Twitter push. I first googled on Bing and got this post. I made a quick implementation of my IAlert Interface like so:

public void Send()
{

    HttpWebRequest request = (HttpWebRequest)HttpWebRequest.Create(_twitterUri);
    request.Credentials = new NetworkCredential(_twitterUserId, _twitterPassword);
    request.Timeout = 5000;
    request.Method = "POST";
    request.ContentType = "application/x-www-form-urlencoded";
    using (Stream requestStream = request.GetRequestStream())
    {
        using (StreamWriter streamWriter = new StreamWriter(requestStream))
        {
            streamWriter.Write(_message);
        }
    }
    WebResponse response = request.GetResponse();
}

However, I get a 401

image

it looks like it is an old implementation of the Twitter API.  I then went to this page and it looks like I can’t do basic HTTP authentication with Twitter – I have to use OAuth.  Twitter’s help page recommended using the C# Twitterizer library.  I went ahead and and installed it from NuGet

 

image

I then re-wrote the send method to use the Twitterizer:

public void Send()
{
    OAuthTokens tokens = new OAuthTokens();
    tokens.ConsumerKey = _consumerKey;
    tokens.ConsumerSecret = _consumerSecret;
    tokens.AccessToken = _accessToken;
    tokens.AccessTokenSecret = _accessTokenSecret;

    TwitterResponse<TwitterStatus> tweetResponse = TwitterStatus.Update(tokens, _message);
}

 

Alas, when I re-ran my unit tests

image

Ugh – I guess I need to add a reference to my test project.  Sure enough, that did the trick:

image

Now I am getting this message back from Twitter

image

I then when to my account settings for the app on Twitter and changed it from Read only to full control:

image

Hit save, regenerated the keys, and still got the same message

I then stumbled onto this.  I then restarted, changed the text of my integration test and voila (that’s French for “Whoop-de-do")

image

Polymorphic Dispatch, Open/Closed, Notification Part2

I received plenty of great feedback about this post on polymorphic dispatch, open/closed principle, and notifications.  I have made the following changes based on the comments

1) Send() should have the message as its argument so that for each message the collection does not have to be recreated.  The local variable that holds the message goes away:

public interface IAlert
{
    void Send(String message);
}

And an implementation looks like:

 

public void Send(String message)
{
    if (String.IsNullOrEmpty(message))
        throw new AlertsException("message cannot be null or empty.");

    SmtpClient smtpClient = new SmtpClient();
    MailMessage mailMessage = new MailMessage(_from, _to, _subject, message);
    smtpClient.Send(mailMessage);
}

2) I should not use an abstract base for the mail – because it tired me to a specific implementation (need a TO, FROM, SUBJECT).  Also, I should be favoring interfaces over abstract classes.  The benefit of reduction of code by using that abstract base is more than offset by the specific implementation and the making it harder to build extensible emails in the future.

3) My Twitter implementation was based on Twitter APIs for 2 years ago.  Twitter now requires OAuth.  A full blog post about hooking up to Twitter is found here.

4) Even though I am following the Open/Closed Principle (OCP) for notifications, I still have a problem.  If I add a new notification to the project, I can add the notification class and AlertFactory does not change (follows OCP).  However, the SqlAlerteeProvider does have to change (violation of OCP) to account for the new kind of notification.  For example:

public Alertee GetAlertee(Int32 alerteeId)
{
    Alertee alertee = new Alertee();
    alertee.AlerteeId = 1;
    alertee.FirstName = "Test";
    alertee.LastName = "Test";
    alertee.AlertList.Add(new LocalEmailAlert("Test","Test","Test"));
    alertee.AlertList.Add(new TwitterAlert());
    //Add new kind of Alert here
    return alertee;
}

I then thought about how to add the alert and have the factory not change.  I first looked at the Factory Pattern in Design Patterns.  This was no help because they too violate the OCP when they create the specific implementations of the subclasses (Creator class on p. 111)

Product* Creator::Create (ProductId id)

{

if(Id == MINE) return new MyProduct;

if(Id==YOURS) return new YourProduct

etc…

}

I also realized that I have been using the words “factory” wrong  my Factories are usually Builders.  In any event, the builder pattern also creates specific concrete classes and would therefor have to change as new types of alerts are added to the project.

I then realized that any concrete implementation will violate the OCP.  I see two ways out of the problem.  I could either make the individual objects responsible for their own creation (bad idea) or use an abstract factory pattern (GoF p.87) (not an all together a bad idea).  The abstract factory implementation needs to be alert specific.  So if you have TwitterAlert, you have a TwitterAlertFactory that follows the interface of the IFactory pattern.  The difference with my current implementation (SqlAlerteeProvider) is about implementing the object, not about implementing the data store.

Assuming this is right, what it means that every new alter that I add to the project is actually 2 classes.  Class 1 is the implementation of alert that follows IAlert and Class 2 is the implementation of alert construction that follows IAlertFactory. 

I don’t like this solution.  The problem is that I still can’t actually create alerts and assign them to users without changing code – now that code can be external (in Main) or in yet a third class that joins Alerts and AlertFactories together.   I suppose I can use a DI container but I will still need to alter the .config file and have a redeploy, so I get no practical value-added.  Just by moving something higher up in the call tree does not make it follow OCP.  The main function will still have to change and be aware of the types of alerts out there. 

I think I will go back to the provider model because it follows the MSFT patterns of use.  If I add a new alert, the Alert has to be added and the AlertProvider will have to change.  Not pure OCP, but closer – and the code is much cleaner than the typical if..then switch…case spaghetti that you typically find in most projects.

Polymorphic Dispatch, Open/Closed Principle, and Notifications

I started on working on a new application last week – one that will read data from various sources, analyze the data, and then notify interested parties about the results of the analysis.  I started working on the notification piece first.  My first premise is that there can be an variety of notification methods and an infinite number of providers of the methods.  For example, a person can receive a notification via an email, a text, a phone call, an IM, an audio signal from their phone app, Twitter, Skype, Facebook, etc…   Each of these methods can be served up by a host of providers.  Since these providers change so quickly and new mechanisms arrive (and quickly become old mechanisms), it makes sense that this part of my application should be extensible as possible.

After trying a variety of scenarios and objects, I decided to concentrate on the core Interface – which is the Alert.  I created an interface like so:

public interface IAlert
{
    void Send();
}

I then created a class called Alertee that contains a collection of IAlerts with some other properties:

public class Alertee
{
    public Alertee()
    {
        this.AlertList = new List<IAlert>();
    }

    public Int32 AlerteeId { get; set; }
    public String FirstName { get; set; }
    public String LastName { get; set; }
    public List<IAlert> AlertList { get; set; }

}

Notice the Command/Query separation.   This class is a data structure that only has public properties.  I initially toyed with calling the Alertee “User” but “User” is such an overused word – most every implementation of IAlert already has a “User” class – and I am a big believer of domain-unique language (and I hate fully-qualifying my Instances) so I chose the less ambiguous if-not-a-real-word: “alertee”.

I then created an AlertFactory (a command class) like so:

public class AlertFactory
{
    public void SendAlerts(List<Alertee> alertees)
    {
        foreach (Alertee alertee in alertees)
        {
            foreach (IAlert alert in alertee.AlertList)
            {
                alert.Send();
            }
        }
    }
}

Notice that by using the IAlert interface, the dependency is inverted and that Alert Factory delegates responsibility of the actual alert implementation via calling the alert.Send().

With the interface, the POCO, and the Factory class ready, I then started implementing the different kinds of alerts.  My first stop was an email alert from the local SMTP server.  I coded this class like so:

public class LocalEmailAlert: IAlert
{
    private String _from = String.Empty;
    private String _to = String.Empty;
    private String _subject = String.Empty;
    private String _body = String.Empty;

    public LocalEmailAlert(String from, String to, String subject, String body)
    {
        if (String.IsNullOrEmpty(from))
            throw new AlertsException("from cannot be null or empty.");
        if (String.IsNullOrEmpty(to))
            throw new AlertsException("to cannot be null or empty.");
        if (String.IsNullOrEmpty(subject))
            throw new AlertsException("message cannot be null or empty.");
        if (String.IsNullOrEmpty(body))
            throw new AlertsException("message cannot be null or empty.");

        _to = to;
        _from = from;
        _subject = subject;
        _body = body;
    }

    public void Send()
    {
        SmtpClient smtpClient = new SmtpClient();
        MailMessage mailMessage = new MailMessage(_from, _to, _subject, _body);
        smtpClient.Send(mailMessage);
    }
}

A couple of things to note.  All of the values that the SMTP needs to use are passed in via the constructor (constructor injection) and these values are stored in private fields.  Also, the values are verified in the constructor and a generic exception is thrown.

I then went to one of the million email providers out there and picked one at random: MailJet.  I then coded up a MailJetEmailProvider like so:

public class MailJetEMailAlert: IAlert
{
    private String _mailServerName = "in.mailjet.com";
    private Int32 _mailServerPort = 465;
    private String _apiKey = String.Empty;
    private String _secretyKey = String.Empty;
    private String _from = String.Empty;
    private String _to = String.Empty;
    private String _subject = String.Empty;
    private String _body = String.Empty;

    public MailJetEMailAlert(String from, String to, String subject, String body)
    {
        if (String.IsNullOrEmpty(from))
            throw new AlertsException("from cannot be null or empty.");
        if (String.IsNullOrEmpty(to))
            throw new AlertsException("to cannot be null or empty.");
        if (String.IsNullOrEmpty(subject))
            throw new AlertsException("message cannot be null or empty.");
        if (String.IsNullOrEmpty(body))
            throw new AlertsException("message cannot be null or empty.");

        _to = to;
        _from = from;
        _subject = subject;
        _body = body;
    }

    public void Send()
    {
        MailMessage mailMessage = new MailMessage();
        mailMessage.From = new MailAddress(_from);
        mailMessage.To.Add(new MailAddress(_to));
        mailMessage.Body = _body;
        mailMessage.Subject = _subject;

        SmtpClient client = new SmtpClient(_mailServerName, _mailServerPort);
        client.EnableSsl = true;
        client.Credentials = new NetworkCredential(_apiKey, _secretyKey);
        client.Send(mailMessage);
    }
}

Notice that the constructor is exactly the same as the local email implementation.  The Mail-Jet specific code/fields are assigned locally and are unique only to the Mailjet class.  I suppose that I could pass those elements in via the constructor and then keep the values in the .config file – but that didn’t seem right to me.  I also could access the config file directly from this class  but that introduces an unneeded dependency – now this class has to worry about the config file (see if it exists, if the section is there, etc…) which is another reason that the class has to change – a violation of the Single Responsibility Principle.  The more I code, the more I think that the config file should be accessed only once and in 1 place (in Main) and those values are then passed into the classes that need it.  That is a blog post for another time – and certainly is against what Microsoft has recommended for many years.

In any event, I don’t think that having the MailJetspecific code embedded into the class is a bad idea  in fact I think it is a good idea.  If these values changes, this class has to be recompiled and redeployed (so what, this isn’t java where the calling assembly has to be recompiled) compared to having the UI being mail-jet aware (via its config) and then having the config file redeployed.  I would rather have a cleaner separation of concerns and have a recompile than a muddied SOC and no recompile.  Both scenarios need to be redeployed anyway.

Back to Mail, I then realized that the mail classes could derive from an abstract base class like so:

public abstract class MailAlert
{

    private String _from = String.Empty;
    private String _to = String.Empty;
    private String _subject = String.Empty;
    private String _body = String.Empty;

    public MailAlert(String from, String to, String subject, String body)
    {
        if (String.IsNullOrEmpty(from))
            throw new AlertsException("from cannot be null or empty.");
        if (String.IsNullOrEmpty(to))
            throw new AlertsException("to cannot be null or empty.");
        if (String.IsNullOrEmpty(subject))
            throw new AlertsException("message cannot be null or empty.");
        if (String.IsNullOrEmpty(body))
            throw new AlertsException("message cannot be null or empty.");

        _to = to;
        _from = from;
        _subject = subject;
        _body = body;
    }

}

The problem is that the derived class still needs to access these private fields – the protected scope solves that:

protected String _from = String.Empty;
protected String _to = String.Empty; 
protected String _subject = String.Empty;
protected String _body = String.Empty;

So the next problem is abstract base class does not have a constructor that takes zero arguments  which is required.  So I added a default empty constructor like so:

public MailAlert():
    this(String.Empty,String.Empty,String.Empty,String.Empty)
{

}

It now complies and I removed the duplicative code.  The sub-lesson here is that, as rule, don’t start with an abstract class.  Start with implementations and if there is lots of repetitive code, the consider refactoring to an abstract class.  Your covering unit tests will tell you if your refactor is wrong  and you are using unit tests, aren’t you?

I then tackled Text pushing via CDyne.  I added a reference to the CDyne Service (I love SOAP) in Visual Studio, received a developer key from CDyne, and wrote the following implementation:

public class CDyneTextAlert: IAlert
{
    private String _phoneNumber = String.Empty;
    private String _message = String.Empty;
    private Guid _licenseKey = new Guid("XXXXXXXXXXXXXXXXXXXXXXX");
    
    public CDyneTextAlert(String phoneNumber, String message)
    {
        if (String.IsNullOrEmpty(phoneNumber))
            throw new AlertsException("phoneNumber cannot be null.");

        if (String.IsNullOrEmpty(message))
            throw new AlertsException("message cannot be null.");

        _phoneNumber = phoneNumber;
        _message = message;
    }

    public void Send()
    {
        IsmsClient client = new IsmsClient();
        SMSResponse response = client.SimpleSMSsend(_phoneNumber, _message, _licenseKey);
    }
}

I then added a Twitter push like so:

public class TwitterAlert: IAlert
{
    private String _twitterUri = "http://twitter.com/statuses/update.json";
    private String _twitterUserId = String.Empty;
    private String _twitterPassword = String.Empty;
    private String _message = String.Empty;

    public TwitterAlert(String message)
    {
        if (String.IsNullOrEmpty(message))
            throw new AlertsException("message cannot be null.");
        
        this._message = message;
    }

    public void Send()
    {

        HttpWebRequest request = (HttpWebRequest)HttpWebRequest.Create(_twitterUri);
        request.Credentials = new NetworkCredential(_twitterUserId, _twitterPassword);
        request.Timeout = 5000;
        request.Method = "POST";
        request.ContentType = "application/x-www-form-urlencoded";
        using (Stream requestStream = request.GetRequestStream())
        {
            using (StreamWriter streamWriter = new StreamWriter(requestStream))
            {
                streamWriter.Write(_message);
            }
        }
        WebResponse response = request.GetResponse();
    }
}

I then added a Phone notification from CDyne:

public class CDynePhoneAlert: IAlert
{
    String _phoneNumber = String.Empty;
    String _message = String.Empty;
    String _callerId = String.Empty;
    String _callerName = String.Empty;
    String _voiceId = String.Empty;
    String _licenseKey = "XXXXXXXXX";

    public CDynePhoneAlert(String phoneNumber, String message, String callerId, String callerName, String voiceId)
    {

        if (String.IsNullOrEmpty(phoneNumber))
            throw new AlertsException("phoneNumber cannot be null.");

        if (String.IsNullOrEmpty(message))
            throw new AlertsException("message cannot be null.");

        if (String.IsNullOrEmpty(callerId))
            throw new AlertsException("callerId cannot be null.");

        if (String.IsNullOrEmpty(callerName))
            throw new AlertsException("callerName cannot be null.");

        if (String.IsNullOrEmpty(voiceId))
            throw new AlertsException("voiceId cannot be null.");

        _phoneNumber = phoneNumber;
        _message = message;
        _callerId = callerId;
        _callerName = callerName;
        _voiceId = voiceId;

    }


    public void Send()
    {
        PhoneNotifySoapClient client = new PhoneNotifySoapClient("PhoneNotifySoap");
        NotifyReturn callReturnValue = client.NotifyPhoneBasic(_phoneNumber, _message, _callerId, _callerName, _voiceId, _licenseKey);
    }
}

I noticed that the message variable is being passed into the constructor in every IALert implementation so perhaps that should be added to the interface definition Send(String message) but then realized that each implementer might have its own requirements on the message.  For example, Twitter needs to check the number of characters:

public TwitterAlert(String message)
{
    if (String.IsNullOrEmpty(message))
        throw new AlertsException("message cannot be null.");

    if(message.Length > 140)
        throw new AlertsException("message is too long for Twitter.");
    
    this._message = message;
}

Also, what happens when the message is no longer a string – perhaps a .mp3 file with an recorded voice message to be sent to the phone?  Then the Send() method would have to overload an audioStream argument.

So instead of trying to build some class (or worse, some enum) of the different types messages that are passed in, I left it to the specific implementation.  This makes sense of the fluid nature of the distribution mechanisms in today’s technology landscape  – the API needs to be as flexible as possible.

So far, I have a solution that follows the Open/Closed principle.  As new distribution mechanisms come available, I just need to add a new class that implements the IAlert interface, add it to the Alertee’s Alerts collection, and it implements.  No other code needs to change.  I then ran into a bump – adding an Alert to the Alertee’s Alerts collection.

I added 1 more class to the project  – a Provider that creates alertes with instantiated Alerts.  I added a SqlAlerteeProvider figuring I will store each Alertee’s desired Alert mechanism (0 to infinity) in a Sql Server database so it looks like so:

public class SqlAlerteeProvider
{
    public Alertee GetAlertee(Int32 alerteeId)
    {
        return null;
    }

    public List<Alertee> GetAllAlertees()
    {
        return null;
    }

    public void UpdateAlertee(Alertee alertee)
    {

    }

    public void InsertAlertee(Alertee alertee)
    {

    }

    public void DeleteAlertee(Alertee aleertee)
    {

    }
}

However, I am deferring the implementation of the data store for as long as possible.  Who knows, I may find a different place to store the data.

So does application follow the open/closed principle?  It does up to the point of the Alertee Provider.  If a new alert needs to be created, a new class is added (ok with O/C) AND the GetAlertee code needs to be changed to account for the new provider (new fields in the database, etc..) (not OK with the O/C).  I suppose I can dig into making the provider follow the O/C, but that is a task for another day.

The important thing is that the calling application DOESN’T change – It just keeps calling SendAlerts() on the AlertFactory regardless of what else happens.