# Should I split code into smaller functions? Advice on layout

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

{
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

//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_Dttm = DateTime.Now;
dc.TestProjectCommandMessages.InsertOnSubmit(msg);
dc.SubmitChanges();
}
else
{
LatestCommandLabel.Text = "No Commands Available";
}
}

protected void GetCommandButton_Click(object sender, EventArgs e)
{
ReturnValue();
}


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

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

return command;
}

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

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

• 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. – user3284707 May 23 '14 at 10:13