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

Returning a large number of values from a thread

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

Problem

I have an application in which I am querying an XML web service continuously every 2 seconds in a thread. The returned XML is very big and I am retrieving lots of stuff from them using XPath and plugging the values into labels in the GUI, using delegates for thread safety.

But the way I've done this, it looks bizarre to me, and I cannot think out of the box and find a way to optimize the code. It's working perfectly but I think there must be a better way to do this.

```
public void GetConditions(object activeDs)
{
string ds = (string)activeDs;
XPathDocument doc;
XmlNamespaceManager ns;
XPathNavigator navigator;
XPathNodeIterator nodes;
XPathNavigator node;
Thread unitThread = new Thread(new ParameterizedThreadStart(SetUnits));
unitThread.Start(ds);
while (contFlag)
{
try
{
doc = new XPathDocument(ds + "/current");
navigator = doc.CreateNavigator();
navigator.MoveToFirstChild();
string uri = navigator.LookupNamespace("");
ns = new XmlNamespaceManager(navigator.NameTable);
ns.AddNamespace("m", uri);

string devName = xmlParser.GetXmlValues(ns, "//m:Devm", "name", navigator);
tsa.SetText(devName, currDevName);

string execStatus = xmlParser.GetXmlValues(ns, "//m:Exec", navigator);
SetExecutionStatus(execStatus);

string spndlSpd = xmlParser.GetXmlValues(ns, "//m:Spieed", "subType", "ACTUAL", navigator);
double spndlSpdDbl; double.TryParse(spndlSpd, out spndlSpdDbl);
tsa.SetText(spndlSpdDbl.ToString("0.000"), spndSpdLbl);

string spndlSpdOvr = xmlParser.GetXmlValues(ns, "//m:Spieed", "subType", "OVERRIDE", navigator);
double spndlSpdOvrDbl; double.TryParse(spndlSpdOvr, out

Solution

First and largest thing I see here is the size of this method. When you're looking at a block of code and having difficulty recognizing how to simplify it, think of it like a bunch of data that you want to simplify, first thing you do is look for patterns by breaking the data up into pieces you can quantify. In your case this means breaking out blocks of the code into descriptively named methods.

I would break up all those parse and tsa.SetText things you have into seperate methods (assuming your xmlParser object is a member level variable):

SetCurrentDevName();
SetExecutionStatus(); // this can do the string parsing on it's own from the member level xmlParser
SetSpndlGibberishAmbiguouslyNamedThing(); // spndlSpd is not a descriptive name for anything
SetSpndlSpdOvrDbl();
SetFeedRateDouble(); // I think that's what fdrtdbl is? Who knows
SetFeedRateOverDouble();
SetModeText();
SetProgText();
SetPartCount();


Next, you have a try block without a catch or finally after it, this serves no purpose unless I'm missing something..

Lastly, make your whole nested while block a single seperate method.

Breaking things up like this into seperate methods you will probably begin to see patterns emerge and realize more ways to simplify this.

Edit:
and as mentioned earlier, use LINQ and XDocuments, far far simpler than the old xmldocument/xpathnavigator stuff.

Code Snippets

SetCurrentDevName();
SetExecutionStatus(); // this can do the string parsing on it's own from the member level xmlParser
SetSpndlGibberishAmbiguouslyNamedThing(); // spndlSpd is not a descriptive name for anything
SetSpndlSpdOvrDbl();
SetFeedRateDouble(); // I think that's what fdrtdbl is? Who knows
SetFeedRateOverDouble();
SetModeText();
SetProgText();
SetPartCount();

Context

StackExchange Code Review Q#4030, answer score: 6

Revisions (0)

No revisions yet.