Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

new wxWidget(wxSmith) project

(1/6) > >>

killerbot:
I just created a new wxWidgets project and choose wxSmith as the gui builder. I choose the dialog based app.
On linux !
Unfortunately it doesn't compile, probably doesn't know where the headers are, seems this is not dealed with by the wizard ??
[Edit] : fixed : I can build now my build options needed to be changed to `wx-config-2.8 --cflags` --> append of -2.8

But I have some important remarks with the generated code, could one of the wizards gurus adopt it please.

In total 5 files are generated (I called my project wxSmith1) :

1) wxSmith1App.h
this header files is including precompiled headers, one should not to do in header files !!
So I would suggest to remove the following construct :

--- Code: ---#include <wx/wxprec.h>

#ifdef __BORLANDC__
    #pragma hdrstop
#endif

#ifndef WX_PRECOMP
    #include <wx/wx.h>
#endif

--- End code ---

by just :

--- Code: ---#include <wx/app.h>

--- End code ---

Remark : I never got the choice to say in I want pch or not !!!
Question : is that _BORLANDC_ pragma really needed. I just looks and feels plain ugly ?

2) wxSmith1App.cpp
- again that _BORLANDC_ stuuf is there --> needed (note : maybe here, but for sure not in the header file ?)
- do NOT use NULL, that's not standard C/C++ ==> the correct value is : 0


3) wxSmith1Main.h
This is something where wxSmith also is not doing the best it could do: In the header file it is including a bunch of headers which are not needed at that point, forward declaration are sufficient. Those headers are only needed at the cpp.
Could this please be changed, so there are less dependencies in the headers.

So in my example this :

--- Code: ---#include "wxSmith1App.h"

//(*Headers(wxSmith1Dialog)
#include <wx/button.h>
#include <wx/dialog.h>
#include <wx/sizer.h>
#include <wx/statline.h>
#include <wx/stattext.h>
//*)

class wxSmith1Dialog: public wxDialog
{
public:

wxSmith1Dialog(wxWindow* parent,wxWindowID id = -1);
virtual ~wxSmith1Dialog();

private:

//(*Handlers(wxSmith1Dialog)
void OnQuit(wxCommandEvent& event);
void OnAbout(wxCommandEvent& event);
//*)

//(*Identifiers(wxSmith1Dialog)
static const long ID_STATICTEXT1;
static const long ID_BUTTON1;
static const long ID_STATICLINE1;
static const long ID_BUTTON2;
//*)

//(*Declarations(wxSmith1Dialog)
wxBoxSizer* BoxSizer1;
wxStaticText* StaticText1;
wxBoxSizer* BoxSizer2;
wxButton* Button1;
wxStaticLine* StaticLine1;
wxButton* Button2;
//*)

DECLARE_EVENT_TABLE()
};

--- End code ---

could, well actually should become :


--- Code: ---#include "wxSmith1App.h"

//(*Headers(wxSmith1Dialog)
#include <wx/dialog.h>
class wxButton;
class wxBoxSizer;
class wxStaticText;
class wxStaticLine;
class wxCommandEvent;
//*)

class wxSmith1Dialog: public wxDialog
{
public:

wxSmith1Dialog(wxWindow* parent,wxWindowID id = -1);
virtual ~wxSmith1Dialog();

private:

//(*Handlers(wxSmith1Dialog)
void OnQuit(wxCommandEvent& event);
void OnAbout(wxCommandEvent& event);
//*)

//(*Identifiers(wxSmith1Dialog)
static const long ID_STATICTEXT1;
static const long ID_BUTTON1;
static const long ID_STATICLINE1;
static const long ID_BUTTON2;
//*)

//(*Declarations(wxSmith1Dialog)
wxBoxSizer* BoxSizer1;
wxStaticText* StaticText1;
wxBoxSizer* BoxSizer2;
wxButton* Button1;
wxStaticLine* StaticLine1;
wxButton* Button2;
//*)

DECLARE_EVENT_TABLE()
};

--- End code ---

4) wxSmith1Main.cpp
Let's first show the code and then discuss what should be changed :


--- Code: ---#ifdef WX_PRECOMP
#include "wx_pch.h"
#endif

#ifdef __BORLANDC__
#pragma hdrstop
#endif //__BORLANDC__

#include "wxSmith1Main.h"

//helper functions
enum wxbuildinfoformat {
    short_f, long_f };

wxString wxbuildinfo(wxbuildinfoformat format)
{
    wxString wxbuild(wxVERSION_STRING);

    if (format == long_f )
    {
#if defined(__WXMSW__)
        wxbuild << _T("-Windows");
#elif defined(__UNIX__)
        wxbuild << _T("-Linux");
#endif

#if wxUSE_UNICODE
        wxbuild << _T("-Unicode build");
#else
        wxbuild << _T("-ANSI build");
#endif // wxUSE_UNICODE
    }

    return wxbuild;
}

//(*InternalHeaders(wxSmith1Dialog)
#include <wx/bitmap.h>
#include <wx/font.h>
#include <wx/fontenum.h>
#include <wx/fontmap.h>
#include <wx/image.h>
#include <wx/intl.h>
#include <wx/settings.h>
//*)

//(*IdInit(wxSmith1Dialog)
const long wxSmith1Dialog::ID_STATICTEXT1 = wxNewId();
const long wxSmith1Dialog::ID_BUTTON1 = wxNewId();
const long wxSmith1Dialog::ID_STATICLINE1 = wxNewId();
const long wxSmith1Dialog::ID_BUTTON2 = wxNewId();
//*)

BEGIN_EVENT_TABLE(wxSmith1Dialog,wxDialog)
//(*EventTable(wxSmith1Dialog)
//*)
END_EVENT_TABLE()

wxSmith1Dialog::wxSmith1Dialog(wxWindow* parent,wxWindowID id)
{
//(*Initialize(wxSmith1Dialog)
Create(parent,id,_("wxWidgets app"),wxDefaultPosition,wxDefaultSize,wxDEFAULT_DIALOG_STYLE,_T("wxDialog"));
BoxSizer1 = new wxBoxSizer(wxHORIZONTAL);
StaticText1 = new wxStaticText(this,ID_STATICTEXT1,_("Welcome to\nwxWidgets"),wxDefaultPosition,wxDefaultSize,0,_T("ID_STATICTEXT1"));
wxFont StaticText1Font = wxSystemSettings::GetFont(wxSYS_DEFAULT_GUI_FONT);
if ( !StaticText1Font.Ok() ) StaticText1Font = wxSystemSettings::GetFont(wxSYS_DEFAULT_GUI_FONT);
StaticText1Font.SetPointSize(20);
StaticText1->SetFont(StaticText1Font);
BoxSizer1->Add(StaticText1,1,wxALL|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,10);
BoxSizer2 = new wxBoxSizer(wxVERTICAL);
Button1 = new wxButton(this,ID_BUTTON1,_("About"),wxDefaultPosition,wxDefaultSize,0,wxDefaultValidator,_T("ID_BUTTON1"));
BoxSizer2->Add(Button1,1,wxALL|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
StaticLine1 = new wxStaticLine(this,ID_STATICLINE1,wxDefaultPosition,wxSize(10,-1),wxLI_HORIZONTAL,_T("ID_STATICLINE1"));
BoxSizer2->Add(StaticLine1,0,wxALL|wxEXPAND|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
Button2 = new wxButton(this,ID_BUTTON2,_("Quit"),wxDefaultPosition,wxDefaultSize,0,wxDefaultValidator,_T("ID_BUTTON2"));
BoxSizer2->Add(Button2,1,wxALL|wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
BoxSizer1->Add(BoxSizer2,0,wxALIGN_CENTER_HORIZONTAL|wxALIGN_CENTER_VERTICAL,4);
SetSizer(BoxSizer1);
BoxSizer1->Fit(this);
BoxSizer1->SetSizeHints(this);
Connect(ID_BUTTON1,wxEVT_COMMAND_BUTTON_CLICKED,(wxObjectEventFunction)&wxSmith1Dialog::OnAbout);
Connect(ID_BUTTON2,wxEVT_COMMAND_BUTTON_CLICKED,(wxObjectEventFunction)&wxSmith1Dialog::OnQuit);
//*)
}

wxSmith1Dialog::~wxSmith1Dialog()
{
//(*Destroy(wxSmith1Dialog)
//*)
}

void wxSmith1Dialog::OnQuit(wxCommandEvent& event)
{
    Close();
}

void wxSmith1Dialog::OnAbout(wxCommandEvent& event)
{
    wxString msg = wxbuildinfo(long_f);
    wxMessageBox(msg, _("Welcome to..."));
}

--- End code ---

- first of all all headers should be included at the top, not first some code and then some headers. In this case this can lead to compile errors, since when there's no pch -> wx_pch.h not included. And we already encounter some code for using undefined types at that moment : wxString, _T(), and only after these first implementation lines of code we see a next bunch of includes. Finally bringing in wx/intl.h (_T()), but still no wxString. So in this example not all needed headers are included !!!
Short word : this code is wrong.
- so the second bunch of headers should move up, and the headers I removed in 3) should join them in the cpp file, and the construct should be something like this :

#ifdef WX_PRECOMP
#include "wx_pch.h"
#else
 // include here all headers you need and that in the pch case would be brought in by the wx_pch.h (and hope this never changes in the future (extension is ok, but reduction not)
#endif
// include here all other wx headers needed but that are not delivered by wx_pch.h [so they get included always: pch and non-pch]

- I didn't check the rest of the code, but 1 thing I already noticed was no header included for wxMessageBox, so the include of <wx/utils.h> should be added otherwise wcode

5) wxsmith1dialog.wxs
As said, the name of my project was wxSmith1, all previous 4 generated files are CamelCase, only this one is NOT.
For consistency, it would be best it also follow the CamelCase convention ==> wxSmith1Dialog.wxs


Once again I would like to stress : please use headers and there includes correctly. You get less dependencies and better portable code [most of the time for example if the linux build is broken in CB it is due to incorrect header inclusions]. So this is extremely important.

Aside of that : wxSmith rules !!!


EDIT : I would suggest to get rid of that _BORLANDC_ thing, since CB code is purely targeted for GCC. And I really wonder if Borland still needs such an ugly beast.

byo:
Ok, I'll take all those things into consideration :) Very good report indeed.


--- Quote from: killerbot on June 21, 2007, 10:35:54 pm ---Remark : I never got the choice to say in I want pch or not !!!
Question : is that _BORLANDC_ pragma really needed. I just looks and feels plain ugly ?

--- End quote ---

I don't like it either, but don't know what will be the result on borland compiler. In the worst case it will just slow down the compilation. This code is just taken from wx examples so it's the suggested way. But I also would like to get rid of it.


--- Quote ---- do NOT use NULL, that's not standard C/C++ ==> the correct value is : 0

--- End quote ---

Right, it should be easy to find and replace.


--- Quote ---This is something where wxSmith also is not doing the best it could do: I nthe header file it is including a bunch of headers which are not needed at that point, forward declaration are sufficient. Those headers are only needed at the cpp.

--- End quote ---

I'll have to consider what code changes it will require. It may introduce some problems when someone would like to write inline function accessing controls. So I'll probably leave this problem for later.


--- Quote ---- first of all all headers should be included at the top, not first some code and then some headers.

--- End quote ---

True, true...


--- Quote ---Finally bringing in wx/intl.h (_T()), but still no wxString. So in this example not all needed headers are included !!!

--- End quote ---

Actually <wx/intl.h> internally includes <wx/string.h>. And it's certain that at least any other include will also use <wx/string.h> (what can not be said about <wx/intl.h>). But you may be right that this "list" doesn't look complete ;) and including <wx/string.h> shouldn't be time-consuming. So I'll probably add it.


--- Quote ---#ifdef WX_PRECOMP
#include "wx_pch.h"
#else
 // include here all headers you need and that in the pch case would be brought in by the wx_pch.h (and hope this never changes in the future (extension is ok, but reduction not)
#endif
// include here all other wx headers needed but that are not delivered by wx_pch.h [so they get included always: pch and non-pch]

--- End quote ---

Huh, that may be challenging since wxSmith doesn't split headers into pch-included and the rest (I just wonder why there are such headers that are not in pch). Since it may require changes of bigger wxSmith code parts, I'll leave it as post-1.0 task. Currently I'll just threat all headers as non-pch. They sohuldn't slow things down very much since header guard macros simply elliminate everything at preprocessor level.


--- Quote ---- I didn't check the rest of the code, but 1 thing I already noticed was no header included for wxMessageBox, so the include of <wx/utils.h> should be added otherwise wcode

--- End quote ---

Actually, here it would be <wx/msgdlg.h> ;)


--- Quote ---As said, the name of my project was wxSmith1, all previous 4 generated files are CamelCase, only this one is NOT.

--- End quote ---
Not a problem. It followed previous scheme used for class wizard (all file names lower case).


--- Quote ---Once again I would like to stress : please use headers and there includes correctly. You get less dependencies and better portable code [most of the tie for example if the linux build is broken in CB it is due to incorrect header inclusions]. So this is extremely important.

--- End quote ---

Hmm, I agree only partially. Of course I agree that headers should (in most cases) reside at the beginning of file. But forward declaration may cause more problems than few extre includes and for sure it wont do any harm when including external library (forward declarations are required only when there's case of cyclic includes), just a slower compilation.


--- Quote ---Aside of that : wxSmith rules !!!

--- End quote ---

 8) Glad to hear that :)

Regards
  BYO

killerbot:

--- Quote ---
Quote
- I didn't check the rest of the code, but 1 thing I already noticed was no header included for wxMessageBox, so the include of <wx/utils.h> should be added otherwise wcode

Actually, here it would be <wx/msgdlg.h> Wink


--- End quote ---
Correct, my mistake.


EDIT : normally forward declarations won't bring any problems, since for the compiler it's more then enough info, and as such if the full header include would have been there, you create a dependency on those files, if they change everyone using your header also recompiles, where only your implementation should recompile. The user of your hader doesn't care that internally there's a wxButton, but for your implementation this is very important ;-) .

Biplab:

--- Quote from: killerbot on June 21, 2007, 10:35:54 pm ---I just created a new wxWidgets project and choose wxSmith as the gui builder. I choose the dialog based app.
On linux !
Unfortunately it doesn't compile, probably doesn't know where the headers are, seems this is not dealed with by the wizard ??
[Edit] : fixed : I can build now my build options needed to be changed to `wx-config-2.8 --cflags` --> append of -2.8

--- End quote ---

wxWidgets wizard was tested several times on Linux before putting it in to repo. Why didn't you report the issue that time?

FYI, it works perfectly provided you select the appropriate options. Unless you specify exactly what happened with you (your OS, wx installation, errors, etc) I would consider that the wizard works perfectly.

It's a bad gesture from a developer to comment on other people's work by trying it once.


--- Quote from: killerbot on June 21, 2007, 10:35:54 pm ---Remark : I never got the choice to say in I want pch or not !!!
Question : is that _BORLANDC_ pragma really needed. I just looks and feels plain ugly ?

--- End quote ---

Ok, noted. This will be added. _BORLANDC_ pragma is necessary if Borland C++ 5.5 compiler is used.


--- Quote from: killerbot on June 21, 2007, 10:35:54 pm ---EDIT : I would suggest to get rid of that _BORLANDC_ thing, since CB code is purely targeted for GCC. And I really wonder if Borland still needs such an ugly beast.

--- End quote ---

Is it written anywhere? Or am I missing anything?

If you are talking about compiling C::B's code with Borland C++ 5.5 compiler, then you may remove that pragma. BCC 5.5 can't compile Squirrel.

Biplab:
For all users information, wxWidgets wizard is Now Broken.

It'll work with wxSmith. But NOT when you'll be using it without wxSmith.

Edit 1: @Byo, Thanks for the update.  :)  But the problem is wxSmith adds the necessary headers whereas without wxSmith the wizard generated codes become blind.

I'll update the wizard generated codes.

Regards,

Biplab

Navigation

[0] Message Index

[#] Next page

Go to full version