asked on

Loop through text boxes on page but textboxes have varying ID's

I have an array of strings called temp [6,7].

Each subscript will hold the text for a text box from the page.

the text boxes are named like this:

ChestPressSet1Rep ChestPressSet1Wt
ChestPressSet2Rep ChestPressSet2Wt
ChestPressSet3Rep ChestPressSet3Wt

These 6 textboxes are the first row for the array.

This pattern is repeated 6 times for : ShoulderPress
TBarRow
BicepsCurl
TricepsCurl
Squat

NOTE: SetxRep (where x is 1, 2 or 3) and SetyWt(where y is 1,2 or 3) is consistent.

I want to extract the text from the text boxes in an efficient way.
I have tried various looping methodology to go through all the text boxes and add the text values to the specific temp location.

I can just simply do :temp[x,y]=TextBoxName.Text and it works but i just want my code to look a little professional.
I realise a lot of substringing and string checking is going to be involved in this.

heres my code for the loop:
/************************************************//
// CHEST INFORMATION IS BEING PUT INTO ARRAY //
//***********************************************//
      int a=1;
      int b=2;
      TextBox tB=null;
      temp[0,0]=Label4.Text;
      foreach(Control cc in Page.Controls)
            {
            if (cc.GetType().ToString() == "System.Web.UI.WebControls.TextBox")
                  {
                  tB=(TextBox)cc;
                  if(((tB.ID).ToString()).EndsWith("Reps"+a.ToString())&& a<=5)
                        {
                        temp[0,a]=tB.Text.Trim();
                        a=a+2;
                        }

            if((tB.ID).EndsWith("Wt"+b.ToString())&&b<=6)
            {
            temp[0,b]=tB.Text.Trim();
      b=b+2;
            }
      }

                  //************************************************//
                  // SHOULDER INFORMATION IS BEING PUT INTO ARRAY //
                  //***********************************************//
                  temp[1,0]=Label11.Text;
                  TextBox tB2=null;
                  int x=1;
                  int y=2;
                  foreach(Control cc in Page.Controls)
                  {
                        if (cc.GetType().ToString() == "System.Web.UI.WebControls.TextBox")
                        {
                              tB2=(TextBox)cc;
                              if((tB2.ID).EndsWith("Reps"+x.ToString())&& x<=5)
                              {
                                    temp[0,x]=tB2.Text.Trim();
                                    x=x+2;
                              }

                              if((tB.ID).EndsWith("Wt"+y.ToString())&& y<=6)
                              {
                                    temp[0,y]=tB.Text.Trim();
                                    y=y+2;
                              }
                        }
                  }

BurntSky

I agree there has to be a better way, but to answer well, I'll need a little more information. How is the aspx page set up? Are you creating the TextBox controls dynamically? Or are they static? Are the TextBox controls in tables?

Also, I don't mean to be nitpicky, but there are a couple things I don't like about the code you posted (other than the fact that it needs to be completely rewritten):

1. cc.GetType().ToString() == "System.Web.UI.WebControls.TextBox" is NOT a good idea. Avoid hardcoded strings AT ALL COSTS. What happens if in some (albeit unlikely) framework update the TextBox class gets moved from that specific namespace? Your code breaks. The better way to do this comparison is if(cc.GetType() == typeof(TextBox)) { ... }

2. Also, you can cut down some of your explicit casts (which generally aren't a good idea) by doing this:

foreach(Control cc in Page.Controls)
{
TextBox tb = cc as TextBox; // notice the implicit cast
if(tb != null)
{
...
}
}

But anyway, those are just a few design issues to keep in mind, irrelevant to the topic at hand.

deersuper

ASKER

Text boxes are all static and are not in tables
and I like your first suggestion. makes a lot of sense.

I dont understand what you mean by page set up but if you mean how many controls I have and all that sort of thing
then I have the Textboxes i mentioned, one button control to insert all the information via a method call to another class
and some label controls. Apart from that nothing.

deersuper

ASKER

int i=0;
foreach(Control c in Page.Controls)
{
if(c.GetType()==(typeof(TextBox)))
      {
            i++;
      }
}
PrintArray(i);

}

void PrintArray(int i)
{
Response.Write(i);
}

The above code , as per your suggestion, is checking the control type. now logically speaking i should show me the number
of text boxes but when I print it using response I get 1.
which shows that the text boxes are not being recognised.

gregoryyoung

as an extension to burntsky ...

the control relationship needs to be recursive(as usercontrols etc are controls within controls) ...

private int CountControls(ControlCollection controls) {
int i=0;
foreach(Control c in controls)
{
if(c.GetType()==(typeof(TextBox)))
{
i++;
}
if(c.Controls.Count > 0) {
i += CountControls(c.Controls);
}
}
return i;
}

Response.Write(CountControls(Page.Controls));

excuse the poorly formatted code (just typing in here)

BurntSky

Yeah, sorry, I should have mentioned it needs to be recursive. The Page.Controls collection doesn't contain every control at it's first level. It contains collections of collections of collections (and so on) of controls.

BurntSky

When I was asking about how the page is set up, I meant how are the controls on the ASPX page layed out? Are they all in rows? Or are they spread out and surrounded by a series of questions/text/etc? Have you thought about throwing everything into a DataGrid instead of using the TextBoxes?

deersuper

ASKER

datagrid for the while is not an option.
basically I am working on the domain (logic layer) and have left the presentatin layer for later
so the text boxes are only by them selves all arranged in 6 rows with each row having 6 boxes

ASKER CERTIFIED SOLUTION

BurntSky

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

BurntSky

Except change that .ID to .Text (I used .ID to test it cause I didnt feel like typing in all the textboxes.)

jatinderalagh

If you have TextBox with the ID's in some pattern like below one
TextBox1
TextBox2
..
..
TextBox25

then Y not to use the find Control method

for (int i = 1; i < 26; i++)
{
System.Web.UI.WebControls.TextBox ctrl = this.FindControl ("TextBox" +i.ToString()) as System.Web.UI.WebControls.TextBox;
if (ctrl != null)
{
// Do your processing in here
}
}

Cheers
Jatinder

BurntSky

Because FindControl is a recursive method that searches every Control within that instance of the Page class. So every time you call the method it creates a ton of overhead. Just because it's fewer lines of code doesn't always mean it's more efficient =)

jatinderalagh

You are right.
Surely it will be slow but how much i think not more then second in any case.
you can try it running on your PC and check the timing difference.

BurntSky

Yes, that's true. On any reasonably equipped machine you'll never notice the speed difference, but little things like this are what add up to big issues when you're developing very large applications. A second here, a second there and before you know it the application just crawls along. It's always a good idea to choose the most reasonably efficient method.

jatinderalagh

That's what i am trying to say when you are writing a large application they run on good hardware.
So you have to choose out in terms of miliseconds and a good readable code.

BurntSky

<flame>

First of all, I don't know what kind of tiny applications you write, but applications I am required to develop are so extensive that if I even let a millisecond or two slide in every method, the application would take 10 minutes to load up.

I don't think you understand what I was getting at. Sure, his application may be small enough that he won't care about a few milliseconds, or even more, but professional software development is all about mantaining good design standards. It's good to do things the RIGHT way, even if it means a few more lines of code.

And you made a horrible comparison. Choosing between milliseconds and readable code? Are you implying my code is not readable? What, are you new? You can't read a simple nested for loop?

I'm sorry, but I just don't see your logic.

</flame>

gregoryyoung

in agreement with BurnySky ...

Using a single hand written recursive method which returns ALL controls is far more efficient than recursing for each individually

as for code readability ... return it as a controlcollection ...

controlcollection textboxes = GetTextBoxes()
//loop through the collection

is no worse than a loop with find control.

deersuper

ASKER

Thanks Burnt Sky .... this works good !

I see my question started a little discussion ..... hehe !

Thanks for the extensively written code.