问题描述
我在C ++中为Windows创建了一个开源记事本程序，允许使用正则表达式（以及其他一些附加功能）进行搜索和替换。
位于
我正在尝试在我的C ++编程中使用最佳实践，并希望
any任何人都可以给予建议由于代码不仅仅是一页
样本，只需查看其中一个源文件（甚至只是一个函数或
两个）同样受欢迎！
如果您回复，您是否也可以在swx dot ch给我发信息给benjamin dot hanson
谢谢！
-
~Samba，不仅仅是一个低成本的文件和打印机服务器〜
- 让我们OpenSource -
----- =通过Newsfeeds.Com发布，未经审查的Usenet新闻= -----
- 世界排名第一的新闻组服务！
----- ==超过100,000个新闻组--19个不同的服务器！ = -----
Nope（参见FAQ）。
好的，第一个问题：为了下载代码，必须提供一个
用户名和密码。我不能为我的生活记住我最后一次在那个****网站选择了什么。但是有一个快速注册选项
，我只需要提供一个电子邮件地址。好！但是，你提供的
电子邮件已经注册了。
现在查看邮件，看看他们是否已经寄给我我的旧邮件用户名等...
不。一个白痴邮件来自某人引用我的一个usenet帖子（
sci.physics.researc）并且不再添加任何内容;一个尝试过的病毒
（自动删除）;一个垃圾邮件显然来自，
获胜通知，是的。
好的，找到选项，让网站邮寄给我我的旧用户名&密码。
现在再次检查邮件...
不，没什么。
现在再次检查邮件...
不，没什么。
现在再次检查邮件...
不，没什么。
现在再次检查邮件...
不，没什么。
现在再次检查邮件...
不，没什么。
我不会创建一个新的e邮件帐户只是为了下载你的
完整代码，所以我必须检查你在页面上显示的内容。
你首先要做的事情关于内部的状态是Notepad RE是基于CEditView的b $ b。。现在什么是CEditView？由于这是Windows I
启动我的MSDN Library。是的，对，
MFC中有一个CEditView类。但是，MFC？为什么不使用WTL？
*建议：首先说明这是一个MFC应用程序。
有些人可能会说这与C ++无关。但它有。
MFC是C ++中的C伪装。 WTL至少是基于模板的，虽然它是微软的风格，例如不安全的构造和所有这些。
显示的实际代码的第一位是
void CFindReplaceDlg :: OnCheckMatchCase（）
{
m_pParentWnd-> PostMessage（WM_NOTIFY_FIND_REPLACE，eMatchCase，
m_CheckMatchCase.GetCheck（））;
}
现在这是使用Windows API服务来更新（？）
对话框所有者窗口的状态 - 我想。但是使用该服务
（消息发布）似乎是不必要的。它引入了类型
不安全。
最好只在父窗口调用一个方法（或者更受限制的
界面）直接。
接下来，我们有
BOOL CFindReplaceDlg :: OnInitDialog（）
{
CDialog :: OnInitDialog（）;
m_CheckWholeWordOnly.SubclassDlgItem（IDC_CHECK_WHOLE_WORD，this）;
if（m_bInitialRegEx）
{
m_CheckWholeWordOnly.EnableWindow（FALSE）;
}
else
{
m_CheckWholeWordOnly.SetCheck（m_bInitialWholeWordOnly）;
}
m_CheckMatchCase.SubclassDlgItem（IDC_CHECK_MATCH_CASE，this）;
m_CheckMatchCase.SetCheck（m_bInitialMatchCase）;
m_CheckRegEx.SubclassDlgItem（IDC_CHECK_REGEX，this）;
m_CheckRegEx.SetCheck（m_bInitialRegEx）;
if（m_IDD == IDD_FIND）
{
m_RadioUp.SubclassDlgItem（IDC_RADIO_UP，this）;
m_Ra dioDown.SubclassDlgItem（IDC_RADIO_DOWN，this）;
m_RadioDown.SetCheck（TRUE）;
if（m_bInitialRegEx）m_RadioUp.EnableWindow（FALSE）;
m_CheckCloseOnMatch.SubclassDlgItem（IDC_CHECK_CLOSE_ON_MATCH，
this）;
}
else
{
m_EditReplaceText.SubclassDlgItem（IDC_EDIT_REPLACE，this）;
m_CheckReplaceAllLikeNotepad.SubclassDlgItem
（IDC_CHECK_LIKE_NOTEPAD，
this）;
}
m_EditFindText.SubclassDlgItem（IDC_EDIT_FIND，this）;
m_EditFindText.SetWindowText（m_strInitialFind）;
返回TRUE ; //返回TRUE除非你将焦点设置为一个控件
// EXCEPTION：OCX属性页应该返回FALSE
}
我注意到的第一件事是缩进尺寸不一致;以前它
是4，现在它是2.
*建议：使用一致的缩进尺寸。
MFC迫使OnInitDialog混乱;在一个基于MFC的应用程序中，没有什么可以做到这一点
，否则，使用构造函数
进行构建。
*建议：不要使用MFC，它不是类型安全的。
接下来我注意到的是似乎没有检查失败。不是
甚至断言。也许那是'因为所有调用的方法都很好
足以在异常失败的情况下抛出异常，但记住MFC是什么？b $ b就像我更喜欢它。
*建议：检查是否有失败。
我注意到的第三件事是m_IDD == IDD_FIND稍后在
$ b中重复$ b其他代码。
这表示你应该有两个不同的类，也许是从封装_common_事物的类派生的
。
*建议：使用继承来分解常见事物，
并对具有不同
功能的对象使用不同的类。 />
新函数，CNotepadreDoc :: OnNotifyFindReplace。我不会在这里重复
代码，但这只是一个很长的if-then-elseif-elsif-elsif只是检查
a变量'的价值一组可能的（显然）常数
值。现在这在技术上是完全可以的，但是更加清晰
（特别是没有其他条件存在）带有''开关''：
*建议：使用''switch''作为''switch''结构。
新功能，CNotepadreDoc :: OnOpenDocument。
这里你有一些基于_old_
MFC宏的非C ++异常处理。
*建议：使用现代C ++异常处理。甚至
微软在他们的MFC文档中这么说！
*查看ScopeGuard标题（Google），使用ScopeGuard获取
清理你在这里使用try-catch的东西，一般来说（但在这个特定情况下不是
）使用RAII（谷歌）而不是试用。
既然如此，我想我已经做了足够的努力，以正确的方向改变我的情绪状态，并在短时间内完成！
干杯，
- Alf
-
A：因为它混乱人们通常阅读文字的顺序。
问：为什么这么糟糕？
A：热门帖子。
问：usenet和电子邮件中最烦人的是什么？
使用Google我发现的第一个真实参考是挪威语
[no。 it.programmering.c ++]常见问题（我参与其中），
< url：http：//utvikling.com/cppfaq/04/04/02/> ;;看点D.
-
答：因为它弄乱了人们通常阅读文字的顺序。
Q ：为什么这么糟糕？
A：热门帖子。
问：usenet和电子邮件中最烦人的是什么？
I have created an open source Notepad program for Windows in C++ that allows
search and replace using regular expressions (and a few other extras). It
is located at http://www.codeproject.com/cpp/notepadre.asp
I''m trying to use best practice in my C++ programming and would appreciate
any advice anyone can give. As code is far more than just a one page
sample, just a review of one of the source files (or even just a function or
two) is equally welcome!
If you reply, could you also mail me at benjamin dot hanson at swx dot ch
Thanks!
--
~ Samba, more than a low cost File and Printer server ~
-- Let us OpenSource --
-----= Posted via Newsfeeds.Com, Uncensored Usenet News =-----
http://www.newsfeeds.com - The #1 Newsgroup Service in the World!
-----== Over 100,000 Newsgroups - 19 Different Servers! =-----
Nope (see the FAQ).
Okay, first problem: in order to download the code one must provide a
username and password. I can''t for the life of me remember what I chose
last time at that ****ing site. But there is a quick-register option
where I only have to supply an e-mail address. OK! But alas, "The
email you supplied has already been registered".
Now checking the mail to see if they''ve sent me my old username etc...
Nope. One idiot-mail from someone quoting a usenet posting of mine (in
sci.physics.researc) and adding nothing more; one attempted virus
(automatically removed); one spam apparently from gl**********@jumpy.it,
"Winning Notice", yeah right.
Okay, found option to have the site mail me my old username & password.
Now checking mail again...
Nope, nothing.
Now checking mail again...
Nope, nothing.
Now checking mail again...
Nope, nothing.
Now checking mail again...
Nope, nothing.
Now checking mail again...
Nope, nothing.
Well I ain''t gonna create a new e-mail account just to download your
complete code, so I''ll have to review just what you display on the page.
The first thing you state about the internals is that "Notepad RE is
CEditView based". Now what is "CEditView"? Since this is Windows I
fire up my MSDN Library. Yeah, right, there is a CEditView class in
MFC. But, MFC? Why don''t you use WTL?
* Recommendation: state first of all that this is an MFC app.
Some folks might say that has nothing to do with C++. But it has.
MFC is C in C++ disguise. WTL is at least template-based, although
it''s Microsoft style with e.g. unsafe construction and all that.
The first bit of actual code displayed is
void CFindReplaceDlg::OnCheckMatchCase ()
{
m_pParentWnd->PostMessage (WM_NOTIFY_FIND_REPLACE, eMatchCase,
m_CheckMatchCase.GetCheck ());
}
Now this is using a Windows API service in order to update (?) the
dialog owner window''s state -- I think. But using that service
(message posting) seems to be unnecessary. And it introduces type
unsafety.
Better just call a method on the parent window (or a more restricted
interface) directly.
Next, we have
BOOL CFindReplaceDlg::OnInitDialog()
{
CDialog::OnInitDialog ();
m_CheckWholeWordOnly.SubclassDlgItem (IDC_CHECK_WHOLE_WORD, this);
if (m_bInitialRegEx)
{
m_CheckWholeWordOnly.EnableWindow (FALSE);
}
else
{
m_CheckWholeWordOnly.SetCheck (m_bInitialWholeWordOnly);
}
m_CheckMatchCase.SubclassDlgItem (IDC_CHECK_MATCH_CASE, this);
m_CheckMatchCase.SetCheck (m_bInitialMatchCase);
m_CheckRegEx.SubclassDlgItem (IDC_CHECK_REGEX, this);
m_CheckRegEx.SetCheck (m_bInitialRegEx);
if (m_IDD == IDD_FIND)
{
m_RadioUp.SubclassDlgItem (IDC_RADIO_UP, this);
m_RadioDown.SubclassDlgItem (IDC_RADIO_DOWN, this);
m_RadioDown.SetCheck (TRUE);
if (m_bInitialRegEx) m_RadioUp.EnableWindow (FALSE);
m_CheckCloseOnMatch.SubclassDlgItem (IDC_CHECK_CLOSE_ON_MATCH,
this);
}
else
{
m_EditReplaceText.SubclassDlgItem (IDC_EDIT_REPLACE, this);
m_CheckReplaceAllLikeNotepad.SubclassDlgItem
(IDC_CHECK_LIKE_NOTEPAD,
this);
}
m_EditFindText.SubclassDlgItem (IDC_EDIT_FIND, this);
m_EditFindText.SetWindowText (m_strInitialFind);
return TRUE; // return TRUE unless you set the focus to a control
// EXCEPTION: OCX Property Pages should return FALSE
}
The first thing I notice is inconsistent indentation size; previously it
was 4, now it''s 2.
* Recommendation: use consistent indentation size.
The OnInitDialog mess is forced upon you by MFC; nothing much one can do
about that in an MFC-based application, but otherwise, use constructors
for construction.
* Recommendation: don''t use MFC, it''s not type-safe.
Next thing I notice is that there seems to be no failure checking. Not
even asserts. Perhaps that''s because all those methods invoked are nice
enough to throw exceptions if they fail, but remembering what MFC was
like I rather doubt it.
* Recommendation: do check for failures.
Third thing I notice is that m_IDD == IDD_FIND is repeated later on in
other code.
This suggest that you should really have two different classes, perhaps
derived from a class that encapsulates the _common_ things.
* Recommendation: do use inheritance to factor out common things,
and do use different classes for objects with different
functionality.
New function, CNotepadreDoc::OnNotifyFindReplace. I''ll not repeat the
code here, but it''s one long if-then-elseif-elsif-elsif just checking
a variable''s value against a set of possible (apparently) constant
values. Now that''s perfectly OK technically, but it would be more clear
(especially that no other kind of condition is present) with a ''switch'':
* Recommendation: do use ''switch'' for ''switch'' constructions.
New function, CNotepadreDoc::OnOpenDocument.
Here you have some decidedly non-C++ exception handling based on _old_
MFC macros.
* Recommendation: do use modern C++ exception handling. Even
Microsoft says so in their documentation of MFC!
* Check out the ScopeGuard header (Google), use ScopeGuard for the
cleanup things you here use try-catch for, and in general (but not
in this specific case) use RAII (Google) rather than try-catch.
Now that''s that, I think I''ve done enough to change my emotional state a
bit in the right direction, and in a short time!
Cheers,
- Alf
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
Using Google the first real reference I found was the Norwegian
[no.it.programmering.c++] FAQ (which I contributed to), at
<url: http://utvikling.com/cppfaq/04/04/02/>; see point D.
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
这篇关于请求代码审查的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持！