4
\$\begingroup\$

I have the following code. I was wondering if anyone can give me any pointers on how to make this more concise and neater? Should I be splitting this down into smaller functions?

    private void ReturnValue()
    {

        //create our WebService object
        var sample = new TestProjectService.Service1();
        //create our string to hold the Command information returned
        //select which web service to use
        var Command = new SimpleWebServiceApp.TestProjectService.CommandMessages();

        switch (WebServiceRadio.SelectedIndex)
        {
            case 0:
                Command = sample.GetData();
                break;
            case 1:
                Command = sample.GetDataLINQ();
                break;
        }

        //Check if a value is returned
        if (Command != null)
        {
            //Display result on screen
            LatestCommandLabel.Text = String.Format("Last Command Called (Using {0}) : {1}",WebServiceRadio.SelectedItem.ToString(),Command.Command);

            //Get the current logged in username
            string userName = System.Security.Principal.WindowsIdentity.GetCurrent().Name.ToString();

            //Write command to the database
            TestProjectLinqSQLDataContext dc = new TestProjectLinqSQLDataContext();
            TestProjectCommandMessage msg = new TestProjectCommandMessage();
            msg.Command_Type = Command.CommandType;
            msg.Command = Command.Command;
            msg.DateTimeSent = Command.DateTimeSent;
            msg.DateTimeCreated = Command.DateTimeCreated;
            msg.Created_User = userName;
            msg.Created_Dttm = DateTime.Now;
            dc.TestProjectCommandMessages.InsertOnSubmit(msg);
            dc.SubmitChanges();
        }
        else
        {
            LatestCommandLabel.Text = "No Commands Available";
        }
    }

    protected void GetCommandButton_Click(object sender, EventArgs e)
    {
        ReturnValue();
    }
\$\endgroup\$
7
\$\begingroup\$

I would basically write a method wherever you've written a comment on what a block of code does. This allows you to hide the specifics of a particular operation and makes it clearer which code paths are taken and what they do at a high level:

private Service1 GetService()
{
    return new TestProjectService.Service1();
}

private CommandMessages GetCommand(Service1 service)
{
    var command = new SimpleWebServiceApp.TestProjectService.CommandMessages();

    switch (WebServiceRadio.SelectedIndex)
    {
        case 0:
            command = service.GetData();
            break;
        case 1:
            command = service.GetDataLINQ();
            break;
    }

    return command;
}

private void DisplayResult(string text)
{
    LatestCommandLabel.Text = text;
}

private string GetUsername()
{
    return System.Security.Principal.WindowsIdentity.GetCurrent().Name;
}

private void SubmitData(string username, Command command)
{
    var dc = new TestProjectLinqSQLDataContext();

    var msg = new TestProjectCommandMessage
    {
        Command_Type = command.CommandType;
        Command = command.Command;
        DateTimeSent = command.DateTimeSent;
        DateTimeCreated = command.DateTimeCreated;
        Created_User = userName;
        Created_Dttm = DateTime.Now;
    };

    dc.TestProjectCommandMessages.InsertOnSubmit(msg);
    dc.SubmitChanges();
}

private void ReturnValue()
{
    var service = GetService(); // We don't care how we construct the service, just get it
    var command = GetCommand(service); // We don't care what how we get the command, just get it

    if (command != null)
    {
        // Display the result however it needs to be displayed
        DisplayResult(String.Format("Last Command Called (Using {0}) : {1}", WebServiceRadio.SelectedItem.ToString(), command.Command));
        string userName = GetUsername(); // We don't care how we get the username, just get it
        SubmitData(username, command); // We don't care how we submit the data, just submit it
    }
    else
    {
        DisplayResult("No Commands Available");
    }
}

protected void GetCommandButton_Click(object sender, EventArgs e)
{
    ReturnValue();
}
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your time to write this out. I thought it would be better to split things up so thank you for demonstrating how to do this best. \$\endgroup\$ – user3284707 May 23 '14 at 10:13

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.