HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Should I split code into smaller functions? Advice on layout

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
intolayoutsplitadvicesmallershouldcodefunctions

Problem

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();
    }

Solution

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();
}

Code Snippets

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();
}

Context

StackExchange Code Review Q#51492, answer score: 7

Revisions (0)

No revisions yet.