OnStartApplicationsCommand function

The foreach loop seems ridiculously lengthy and I am trying to get rid of it altogether is possible. But there are a lot of things happening in there which I need to know in order to update the UI and create other values. For examples, variables mappedRuntimeName, mappedDeviceName are used for updating the UI and also for creating the RuntimeDetailsobject. The DownloadApplications function also seems to have a lot of parameters.

Does anyone have any suggestions on how to improve the shown OnStartApplicationsCommand function?

```
public bool? OnStartApplicationsCommand()
{
var runCommandSuccessful = true;

//get all the services for download begin
ResolveAndUpdateDependencies();

CancelOperationCommand = new AsyncDelegateCommand(StartApplicationsCommand.Cancel, () => !StartApplicationsCommand.CanExecute(null));
ProjectCommands.CancelOperation.RegisterCommand(CancelOperationCommand);

_eventAggregator.GetEvent().Publish(OutputWindowSourceType.Run);

var activeConfiguration = _configurationService.GetActiveConfiguration();

// Message: >>>>>> Downloading and running applications...
PostOutputMessage(string.Format(ForteDownloadMessages.Default.download_level1_info,
_projectManagerService.GetProjectName(),
_configurationService.GetActiveConfiguration().Name),
OutputWindowMessageType.Info);

var count = 1; //application count
RuntimeDetails lastRuntimeDetails = null; //remember (for disconnecting) where the last application was downloaded to
DownloadedApplications.RemoveAll(application => true); //just remove all

foreach (var application in activeConfiguration.Applications)
{
if (StartApplicationsCommand.CancellationRequested)
{
ProjectCommands.CancelOperation.UnregisterCommand(CancelOperationCommand);
return null;
}

//retreive the fully

//need to close connection when switching runtimes
        //Do this before assigning the request executor so that disconnect is not skipped when runtimes
        //are switched
        if (lastRuntimeDetails != null) //at least one application was already downloaded
            if (lastRuntimeDetails.Ip != runtimeDetails.Ip ||
                lastRuntimeDetails.Port != runtimeDetails.Port)
                _requestExecutor?.Disconnect();

When you nest if statements please use brackets.

You should use what is called a null-safe dereference operator like you suggested in the comments,

if (lastRuntimeDetails != null)
{
    if (lastRuntimeDetails?.Ip != runtimeDetails?.Ip ||
        lastRuntimeDetails?.Port != runtimeDetails?.Port)
    {
        _requestExecutor?.Disconnect();
    }
}

A positive to doing it this way, if for some reason the lastRuntimeDetails.Port property is null but the object is not (or the Ip property) this will not throw a Null Reference Exception, I think that is the biggest positive to this.

if (downloadSuccessful == false)
            runCommandSuccessful = false;

        if (downloadSuccessful == null) //if cancellation requested return null and end
            return null;

you could probably turn this into an else/if statement, because if downloadSuccessful is false, then it is not null, no reason to check for something you already know.

if (downloadSuccessful == false)
{
    runCommandSuccessful = false;
}   
else if (downloadSuccessful == null) //if cancellation requested return null and end
{
    return null;
}

on further though you could just assign runCommandSuccessful directly from downloadSuccessful like this

runCommandSuccessful = downloadSuccessful ?? return null;

using the null-coalescing operator

Naming here could be better,

DownloadedApplications.Add(new DownloadedApplication()
        {
            Application = application,
            Device = matchedConfigurationDeviceModel,
            RuntimeDetails = runtimeDetails,
            Resource = resourceInstance,
            Running = (bool) downloadSuccessful
        });

I assume that DownloadedApplications is a list of DownloadedApplication objects. I think that you change the object name to Application then your DownloadedApplications can be a list of Application objects that have been downloaded

Otherwise the term DownloadedApplication gets rather monotonous, and loses its meaning.

your port variable here isn't very helpful, it just takes up more memory

var port = Convert.ToInt32(matchedConfigurationRuntimeModel.Port);

        var runtimeDetails = new RuntimeDetails()
        {
            DeviceName = matchedConfigurationDeviceModel.Name,
            Platform = mappedDeviceModel.Platform,
            RuntimeType = matchedConfigurationRuntimeModel.Type,
            RuntimeName = matchedConfigurationRuntimeModel.Name,
            Ip = ipAddress, 
            Port = port
        };

you should just assign the Port property directly, you might even be able to get away without converting to an integer, but you will have to test that theory yourself. I don't immediately see any reason that you would need to convert that to an integer, it may already be an integer, you should definitely look into this if you are trying to make your code more efficient.

var runtimeDetails = new RuntimeDetails()
{
    DeviceName = matchedConfigurationDeviceModel.Name,
    Platform = mappedDeviceModel.Platform,
    RuntimeType = matchedConfigurationRuntimeModel.Type,
    RuntimeName = matchedConfigurationRuntimeModel.Name,
    Ip = ipAddress, 
    Port = Convert.ToInt32(matchedConfigurationRuntimeModel.Port)
};

What I would do to shorten the for loop is to consolidate the variables that you are using intermediately to create your RuntimeDetails object. I touched on this briefly and would like to expand the scope of it

You are also using some variables to assign data to a PostOutputMessage I would get rid of those variables, they are several lines away from where they are used as well, that made it hard to tell what they were for.

You should also get rid of the try catch that is commented out, dead code smells

There are a lot of comments, I removed some of the ones I thought were extraneous.

I also removed the count variable, I didn't see it doing anything.

In your switch statement you have a case that doesn't do anything, I would drop this off and let the default grab that case.

```
switch (matchedConfigurationRuntimeModel.Type)
{
case RuntimeType.FORTE:
resourceInstance = _resourceProvider.GetEmbeddedResource(application.Name + "_EMB_RES");
_requestExecutor = _forteRequestExecutor;
break;
case RuntimeType.FBRT:
resourceInstance = _resourceProvider.GetPanelResource(application.Name + "_PANEL_RESOURCE");
_requestExecutor = _fbrtRequestExecutor;

StackExchange Code Review Q#124095, answer score: 5