Re: decouple UI

On 12/23/10 5:12 PM, mp wrote:
I'm playing with a little app to show some values from a spreadsheet(excel)

First i had some code in the form to open and read the excel files

Then I was thinking about the idea of the form being coupled as little as
possible to the rest of the code

So I took the excel stuff out of the form and created an excelReader class
to do all that stuff...

now if i want to show some results in the form, is the following an "ok" way
to do it or is there a better pattern i should be using?

it's clunky but at some point the form has to know what control it want to
display what info there's some inevitable coupling at some point,

The Form sub-class absolutely should know what control it's using to display the info. But only the Form sub-class should know.

On the other hand, it's not clear the Form sub-class should know anything at all about the Excel data. In a true "decoupled" approach such as found in the MVC pattern, the Form sub-class would expose properties that correspond to specific data, and a controller class would handle setting the properties appropriately based on the Excel data (from the reader).

In some cases, it is reasonable to combine the Form and your controller, in which case you very well may find logic within the Form that connects the Excel data to specific controls. This becomes less feasible the more complicated the Form and controller logic get, but for simple UIs it can work fine.

Other comments:

ps i tried to get rid of the switch statement below by using foreach but
found a lable control isn't a control apparently

System.Windows.Forms.Label definitely is a sub-class of System.Windows.Forms.Control. You can see that clearly by looking at the documentation for the Label class.

foreach (Control ctl in this.Controls)
//label is not a control!!!
Debug.Print("Control " + ctl.Name);
if (ctl.Name == "lbl" + rangeName)
ctl.Text = rangeName + ": " + String.Format("{0:
###,###.00}", rng1.Value2);
(no label names are output)

Without a concise-but-complete code example, it's not possible to say for sure what's wrong. But the most likely cause is that your Label instances are not direct children of the Form instance. Instead, they are children of some other container child within the Form.

code snip follows: the form (show value of certain ranges in a lable in the form)
private void btnReadExcel_Click(object sender, EventArgs e)
string filename = @"Path to file.xls";

//open excel file and get reference to specific worksheet
ExcelReader xlReader=new ExcelReader(filename, sheetname);

//range names i want to view
string [] rangeNames = new string[] {"Cash","Stocks", "Bonds" ,
"RealEstate", "Total"};

//show values in lables(or other control later )
for (int i = 0; i< rangeNames.Length; i++)
this.ShowRangeLabel(xlReader, rangeNames[i]);

It would make more sense to declare your "rangeNames" variable as a static class member:

private static string[] rangeNames = …;

That way, it's instantiated only once.

private void ShowRangeLabel( ExcelReader xlReader,string rangeName)
Excel.Range rng1;

rng1 = xlReader.GetRange(rangeName);
MessageBox.Show("Range not found " + rangeName);
switch (rangeName)
case "Cash":
this.lblCash.Text = "Cash: " + String.Format("{0:
###,###.00}", rng1.Value2);
case "Stocks":
this.lblStocks.Text = "Stocks: " + String.Format("{0:
###,###.00}", rng1.Value2);

Another alternative would be to not have the array of names at all. Instead, associate each Label instance with a name in some easy to use way. I would not recommend using the name of the control itself, but if you do then at the very least you should just extract the particular substring from the control's name, rather than keeping a separate list of names and searching a list of controls for each name.

In the Designer, you can set the Tag property of a control, which can be used to store a string such as the name for your reader. IMHO, that's a somewhat better approach than using the control's name.

If you really do want a collection of range names, then IMHO it makes more sense to make that a dictionary, with the key being the range name, and the value being a delegate instance, or perhaps a custom type containing more than one delegate instance if you have more than one operation to associate with the name (e.g. setting a value and retrieving a value). Then you simply look up the delegate and invoke it based on the name. You can enumerate the dictionary members for situations where you want to handle all members, and use the individual keys to get specific data (for example, if you have an indexer on the Form class to extract values for specific range names).


Relevant Pages

  • Re: Userform causes Excel to crash
    ... 1.You may delete the multipage control & recreate new one. ... Shailesh Shah ... If You Can't Excel with Talent, ... Excel does not crash if I include the following code in front of the above ...
  • Re: Need to use VBA to put image on a form
    ... The embedded OLE object with Excel automation sounds workable. ... Earl Kiosterud ... Note: Some folks prefer bottom-posting. ... the OP can place an Image control, ...
  • RE: Usage of serial port in VBA ( exel )
    ... communications control (Microsoft Communications Control, ... Unfortunately Excel makes it almost impossible to do serial ... Software Wedge is an executable program that can pass serial data back ...
  • Re: Word doc in VFP
    ... Or maybe you need revision control system, ... Just save the Word.doc or Excel. ... command button on a form that calls that Word file and opens it up in ... in the begining and then changed to OleBound with general field. ...
  • RE: URGENT- Form design and field tab-order problems
    ... creating an Excel User Form, but this would require you to get into the ... there is a TabIndex property for each control you could set. ... A requester sent several Excel files that have been formatted to print ... The reason the forms are in Excel is that all service techs already ...