General Question ... Class & Members
in [MFC]
|
From: JCO on 9 May 2010 12:44 I understand, however, in my case... if I don't use the pointer (type CEdit), it will return the name of my program. If I use the pointer as in my example, it returns the user's input into the EditBox (which was my goal). "David Ching" <dc(a)remove-this.dcsoft.com> wrote in message news:eR5OtQt7KHA.980(a)TK2MSFTNGP04.phx.gbl... > "JCO" <someone(a)somewhere.com> wrote in message > news:uJXGecs7KHA.5820(a)TK2MSFTNGP04.phx.gbl... >> Joseph, >> I've read every ones comments and I understand that the UpdateData() can >> cause issues. If done your way, does this mean you create a control >> (ctl) variable for every control on your Dialog? This brings me to my >> next issue: >> >> Which method do you recommend to use? >> One: //no control variable used >> CString strName; >> CEdit *pName = (CEdit*) GetDlgItem(IDC_CUSTOMER_NAME); >> >> pName->GetWindowTextw(strName); >> SetCustomerName(strName); >> >> >> Two: //uses control variable >> CString strName; >> ctlEditCutomerName.GetWindowTextW(strName); >> SetCustomerName(strName); >> > > Use Method 1. However, since GetWindowText is a method of CWnd, you don't > nee to cast to CEdit *: > > CWnd *pName = GetDlgItem(IDC_CUSTOMER_NAME); > pName->GetWindowText(strName); // <-- note: no need to call > GetWindowTextW > SetCustomerName(strName); > > > This is important, it is technically illegal to cast the return of > GetDlgItem() to a CWnd derived class like CEdit. This is because the > class returned by GetDlgItem() is *not* a CEdit *, it is a CWnd * because > that's what was constructed when your dialog was initialized. It usually > works out that if the dialog control really is an edit control, that > casting to CEdit * will work but it is not a good idea. > > If you find yourself having to cast in order to call methods declared only > in CEdit (and not in CWnd), use method 2, which constructs a real CEdit > instance to handle the dialog item. > > -- David
From: JCO on 9 May 2010 12:58 Thanks "David Ching" <dc(a)remove-this.dcsoft.com> wrote in message news:eR5OtQt7KHA.980(a)TK2MSFTNGP04.phx.gbl... > "JCO" <someone(a)somewhere.com> wrote in message > news:uJXGecs7KHA.5820(a)TK2MSFTNGP04.phx.gbl... >> Joseph, >> I've read every ones comments and I understand that the UpdateData() can >> cause issues. If done your way, does this mean you create a control >> (ctl) variable for every control on your Dialog? This brings me to my >> next issue: >> >> Which method do you recommend to use? >> One: //no control variable used >> CString strName; >> CEdit *pName = (CEdit*) GetDlgItem(IDC_CUSTOMER_NAME); >> >> pName->GetWindowTextw(strName); >> SetCustomerName(strName); >> >> >> Two: //uses control variable >> CString strName; >> ctlEditCutomerName.GetWindowTextW(strName); >> SetCustomerName(strName); >> > > Use Method 1. However, since GetWindowText is a method of CWnd, you don't > nee to cast to CEdit *: > > CWnd *pName = GetDlgItem(IDC_CUSTOMER_NAME); > pName->GetWindowText(strName); // <-- note: no need to call > GetWindowTextW > SetCustomerName(strName); > > > This is important, it is technically illegal to cast the return of > GetDlgItem() to a CWnd derived class like CEdit. This is because the > class returned by GetDlgItem() is *not* a CEdit *, it is a CWnd * because > that's what was constructed when your dialog was initialized. It usually > works out that if the dialog control really is an edit control, that > casting to CEdit * will work but it is not a good idea. > > If you find yourself having to cast in order to call methods declared only > in CEdit (and not in CWnd), use method 2, which constructs a real CEdit > instance to handle the dialog item. > > -- David
From: JCO on 9 May 2010 12:57 Okay you pretty much answered all of my questions. The question you had about.. SetCustomerName(strName); All this does is copy the name to the data member of the class as shown below m_strClientName = strName; I guess I'm a strong believer in encapsulation... so I use Get & Set statements to access my private data members. Your point to the fact that the size of my app should not matter. It's not that I think it will be to large for a system to run. I'm just being mindful of memory size allotted when someone runs the application. I think that's being respectful. When I run applications that are blotted in size, it makes me wonder why. I do understand what your point will be.... Instead of worrying about the program size, I should worry more about the issues caused from using GetDlgItem() and I should worry about... down the road when modifications to the program is being made.... which is more readable. Thanks again. "Joseph M. Newcomer" <newcomer(a)flounder.com> wrote in message news:ra9cu5pobtap589eighdqkltafp1ll5ckq(a)4ax.com... > See below... > On Sat, 8 May 2010 10:57:11 -0500, "JCO" <someone(a)somewhere.com> wrote: > >>Joseph, >>I've read every ones comments and I understand that the UpdateData() can >>cause issues. If done your way, does this mean you create a control (ctl) >>variable for every control on your Dialog? This brings me to my next >>issue: > **** > It means I create a control variable for every control I care about. > That's a different > statement. For example, for static labels (control ID IDC_STATIC) I don't > bother to > create controls. > **** >> >>Which method do you recommend to use? >>One: //no control variable used >>CString strName; >>CEdit *pName = (CEdit*) GetDlgItem(IDC_CUSTOMER_NAME); > **** > This sucks. It is inappropriate. > **** >> >>pName->GetWindowTextw(strName); >>SetCustomerName(strName); >> >> >>Two: //uses control variable >>CString strName; >>ctlEditCutomerName.GetWindowTextW(strName); > **** > This is the only way I work. Except I would not use GetWindowTextW; that > is the kind of > programming error Intellinonsense tends to introduce. I would have called > GetWindowText. > **** >>SetCustomerName(strName); > **** > You have not specified what SetCustomerName does, or where it is declared. > So I can't > comment on the behavior of this call. > **** >> >>Method two seems easier, however, you must declare the control variable >>(not >>shown) which makes your code larger in size (I realize not much). > **** > Frankly, if you are worried about this kind of issue, you need to get a > grip on life. It > is a completely foolish concern. It means you think code size matters, > and essentially, > code size never matters, not when you have a 2GB virtual address space. > We are no longer > programming PDP-11 minicomputers with 64K of memory, and such concerns are > misplaced. > > Program like it is 2010, not 1975. > **** >>My issue >>... the Dialog I'm working on has 30 CEditBox. Do I really need to create >>30 Control Variables (ex ctlCustomerName) plus 30 member variables (ex >>m_strCutomerName)? If I use the GetDlgItem(), I avoid the 30 Control >>Variables. > *** > Sure. Why not? What possible problem could this cause? The GetDlgItem > requires a cast, > which I consider disastrous. Look at any of the dialogs in any of the > source code I > have. If I have 60 controls I have 60 control variables, so what? I > cannot imagine doing > it any other way. I have one massive dialog that has a couple hundred > control variables. > The GetDlgItem method is antiquated, a throwback to Petzold-style C > programming, and > error-prone. > joe > **** >> >>Thanks for your in-depth response to my original question. >> >> >>"Joseph M. Newcomer" <newcomer(a)flounder.com> wrote in message >>news:1i44u59hf7o13rrovvkbd5lfgd8fo0q0ke(a)4ax.com... >>> I am always offering a contrarian opinion on this: I would NEVER, EVER >>> use >>> a variable to >>> hold a value except in some extremely rare and estoteric situations >>> which >>> I hardly ever >>> encounter, so NEVER, EVER is a pretty good characterization of what I >>> do. >>> >>> I make all the variables "control" variables which are just ways to name >>> the control, and >>> use methods of those variables to extract the data from the control, >>> such >>> as >>> GetWindowText, GetCheck, GetCurSel, etc. >>> >>> I never, ever call UpdateData in a dialog or formview; I think the ONLY >>> valid times this >>> is called is when the framework calls them before OnInitDialog or after >>> OnOK is called, >>> and 99% of the time, I never use this capability either. I seriously >>> preach that the >>> whole DDX mechanism should not be used, EXCEPT for DDX_Control that >>> binds >>> controls to >>> variables, and the entire DDV mechanism should be ignored completely, in >>> favor of >>> intelligent real-time input validation. >>> >>> I generally react to code that arrives on my desk by removing all these >>> variables and >>> removing all UpdateData calls, and replacing them with what I consider >>> sane and robust >>> code. I allow ONLY control variables to exist. >>> On Wed, 5 May 2010 14:23:35 -0500, "JCO" <someone(a)somewhere.com> wrote: >>> >>>>I have a general question concerning members of class and assigning the >>>>content. Particularly if I have a class that, for instance has an >>>>EditBox. >>>>If I use the Wizard, I can create a member data of type "Control" or >>>>"Variable". If I choose variable, the wizard does it's thing. Is this >>>>variable simply used as a conduit? Should it be used simply to set my >>>>actual data member that I created manually in my Class? Example. >>>>m_editClientName was created from the Wizard as type variable CString. >>>>m_strClientName was created by me as part of my private class member. >>>> >>>>Code: >>>>UpdateData(true); >>> **** >>> You can tell this was designed by an amateur because UpdateData takes an >>> argument, true or >>> false, to indicate the direction of flow. If anything resembling >>> intelligent design was >>> used, there would have been methods called ControlsToVariables and >>> VariablesToControls, >>> instead of the non-mnemonic 'true' and 'false' options of UpdateData. >>> You >>> can always tell >>> bad design because it exhibits pathologies like this. I never liked it >>> when I first >>> encountered it, because I had already spent over two decades arguing >>> against this kind of >>> design (it is hard to use, highly error-prone, and basically sucks) >>> ***** >>>>m_strClientName = m_editClientName; //editbox variable is simply >>>>a >>>>conduit to set my my actual member >>> **** >>> I would use >>> m_EditClient.GetWIndowText(m_strClientName); >>> >>> which makes it obvious what is going on; there is never a chance that >>> the >>> variables and >>> the controls are out-of-sync. There is only one truth, the truth in the >>> control. Note >>> that because I rely on the truth of the control, there is never a need >>> to >>> have the string >>> value kept in a member variable at all! In fact, it is not at all clear >>> why you wrote the >>> equivalent of >>> B = A: >>> above, because A already has the value and there is no reason to make a >>> copy of it in B! >>> ***** >>>> >>>>I'm asking the question because sometimes I fill like I'm creating more >>>>data >>>>variables than really needs to be. So, is it good practice to do it the >>>>way >>>>shown above? In reality (I just didn't show it), my data is private >>>>and >>>>I >>>>use Public "Get" & "Set" statements to get & set the variables. So >>>>below >>>>is >>>>the way I've been doing it. >>> **** >>> My opinion: if you have n data variables, you have n-too-many variables. >>> **** >>>> >>>>Code: >>>>UpdateData(true); >>>>SetClientsName( m_editClientName ); >>> **** >>> It is not clear why you need to do this inside the implementation. Not >>> that it is bad, >>> but if the whole point is to copy a private value to a public value, >>> there >>> are serious >>> questions that should be asked, such as why there is even a private copy >>> at all. >>>> >>>>Continued Code: >>>>MyClass::SetClientsName ( CString strName ) >>>>{ >>>> m_strClentName = strName; >>>>} >>> **** >>> What good does it do to set this name in a variable if the variable is >>> not >>> transferred to >>> the control? The whole point of using a setter is that it should >>> actually >>> do something to >>> make this internal copy be the control contents, or the control and the >>> copy are >>> out-of-sync and if the user types something we are going to have >>> problems. >>> >>> Note that doing this right is not always easy or straightforward, but >>> doing it wrong >>> (UpdateData) is really easy and supported by the framework; so it is >>> easy >>> to get wrong. >>> >>> None of my dialogs have ever been so trivial as to be able to use >>> UpdateData. For >>> example, I want to enable the Dothis button when the edit control is >>> non-empty. UpdateData >>> doesn't have any provision for this. >>> >>> We really need to have dialogs support the OnUpdateCommandUI mechanism; >>> but I essentially >>> do that explicitly with my constraint-based model (see my essay on >>> dialog >>> control >>> management on my MVP Tips site) >>> joe >>> **** >>>> >>>>Thanks for your response. >>>> >>>> >>>> >>>> >>> Joseph M. Newcomer [MVP] >>> email: newcomer(a)flounder.com >>> Web: http://www.flounder.com >>> MVP Tips: http://www.flounder.com/mvp_tips.htm > Joseph M. Newcomer [MVP] > email: newcomer(a)flounder.com > Web: http://www.flounder.com > MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Joseph M. Newcomer on 9 May 2010 13:41 See below... On Sun, 9 May 2010 11:44:03 -0500, "JCO" <someone(a)somewhere.com> wrote: >I understand, however, in my case... if I don't use the pointer (type >CEdit), it will return the name of my program. If I use the pointer as in >my example, it returns the user's input into the EditBox (which was my >goal). *** But what I'm saying is that your method of GETTING the "pointer type" is fundamentally flawed! MFC's natural paradigm is to use a control variable, and you don't WANT a pointer type. I would have a variable declared CEdit c_CustomerName; which I would add using the MFC tooling ("add variable"). This will also generate the call in DoDataExchange: DDX_Control(pDX, IDC_CUSTOMER_NAME, c_CustomerName); which binds the control to the variable. Then I would write c_CustomerName.GetWindowText(strName); this is MUCH easier than writing GetDlgItem everywhere and doing casts! AND, if I subclass the control, and change it to CMyEdit c_CustomerName, then I would have to find every GetDlgItem and change the casting, which introduces the possibility of serious errors creeping in. In my model, I change the type of the declaration and never again worry about anything else. joe **** > >"David Ching" <dc(a)remove-this.dcsoft.com> wrote in message >news:eR5OtQt7KHA.980(a)TK2MSFTNGP04.phx.gbl... >> "JCO" <someone(a)somewhere.com> wrote in message >> news:uJXGecs7KHA.5820(a)TK2MSFTNGP04.phx.gbl... >>> Joseph, >>> I've read every ones comments and I understand that the UpdateData() can >>> cause issues. If done your way, does this mean you create a control >>> (ctl) variable for every control on your Dialog? This brings me to my >>> next issue: >>> >>> Which method do you recommend to use? >>> One: //no control variable used >>> CString strName; >>> CEdit *pName = (CEdit*) GetDlgItem(IDC_CUSTOMER_NAME); >>> >>> pName->GetWindowTextw(strName); >>> SetCustomerName(strName); >>> >>> >>> Two: //uses control variable >>> CString strName; >>> ctlEditCutomerName.GetWindowTextW(strName); >>> SetCustomerName(strName); >>> >> >> Use Method 1. However, since GetWindowText is a method of CWnd, you don't >> nee to cast to CEdit *: >> >> CWnd *pName = GetDlgItem(IDC_CUSTOMER_NAME); >> pName->GetWindowText(strName); // <-- note: no need to call >> GetWindowTextW >> SetCustomerName(strName); >> >> >> This is important, it is technically illegal to cast the return of >> GetDlgItem() to a CWnd derived class like CEdit. This is because the >> class returned by GetDlgItem() is *not* a CEdit *, it is a CWnd * because >> that's what was constructed when your dialog was initialized. It usually >> works out that if the dialog control really is an edit control, that >> casting to CEdit * will work but it is not a good idea. >> >> If you find yourself having to cast in order to call methods declared only >> in CEdit (and not in CWnd), use method 2, which constructs a real CEdit >> instance to handle the dialog item. >> >> -- David Joseph M. Newcomer [MVP] email: newcomer(a)flounder.com Web: http://www.flounder.com MVP Tips: http://www.flounder.com/mvp_tips.htm
From: David Ching on 9 May 2010 18:51
"JCO" <someone(a)somewhere.com> wrote in message news:OiYnUb57KHA.420(a)TK2MSFTNGP02.phx.gbl... > I understand, however, in my case... if I don't use the pointer (type > CEdit), it will return the name of my program. If I use the pointer as in > my example, it returns the user's input into the EditBox (which was my > goal). > This can't be right. Both of these lines set strName to exactly the contents of the edit box: CString strName; CEdit *pEditName = (CEdit*) GetDlgItem(IDC_CUSTOMER_NAME); pEditName->GetWindowText(strName); CWnd *pWndName = GetDlgItem(IDC_CUSTOMER_NAME); pWndName->GetWindowText(strName); You might be thinking of: this->GetWindowText(strName); // or simply GetWindowText(strName) which indeed would return the name of your program. -- David |