管理客户关系管理系统中AD用户的代码
管理客户关系管理系统中AD用户的代码
提问于 2016-07-29 10:26:33
我在ASP.NET WebForms (旧式课程)有一个很大的课程。我用的是2012年。
我想评估条件和执行操作-命令如下：
在AD中创建删除在客户关系管理中启用的DesEnable (删除在客户关系管理中)删除许可证
TODO代码：(没有错误处理)
if (ActionTypePage == ActionType.Modification || ActionTypePage == ActionType.MyDataMod)
if (user.Enabled && isEmployer)
{
// 1a) (Employer) NOT exists in AD ==> Error
if (!existsinAD)
{
ShowNotification(msg, CommunicationType.Error, true);
return;
}
// 1b) If Employer exists in AD ==> Enable in CRM (If false, New Request to CRM)
// Enable in CRM (If false, New Request CRM)
}
if (user.Enabled && !isEmployer)
{
// 2a) If NOTEmployer NOT exists in AD ==> Create user in AD y Enable in CRM (If false, New Request CRM)
if (!existeinAD)
{
// Create in AD
// Enable in CRM (If false, New RequestCRM)
}
// 2b) If NOTEmployer exists in AD ==> Warning (Email) y Enable in CRM (If false, New RequestCRM)
if (existeinAD)
{
// Send Email Warning
// Enable in CRM (If false, New RequestCRM)
}
}
// DES-Enable
// 1a) If (Employer) NOT exists in AD ==> Error
// 1b) If (Employer) exists in AD ==> DesEnable in CRM (Delete in CRM) - Delete Licenses CRM
if (!user.Enabled && isEmployer)
{
if (!existsinAD)
{
ShowNotification(msg, CommunicationType.Error, true);
return;
}
// DesEnable in CRM (Delete in CRM)
// Delete Licenses CRM
}
if (!user.Enabled && !isEmployer)
{
// 2a) If NOTEmployer NOT exists in AD ==> Warning (Email) y DesEnable in CRM (Delete in CRM) - Delete Licenses CRM
// Delete Licenses CRM
if (!existeinAD)
{
// Send Email Warning
// DesEnable in CRM (Delete in CRM)
// Delete Licenses CRM
}
// 2b) If NOTEmployer exists in AD ==> Eliminar de AD y DesEnable in CRM (Delete in CRM) - Delete Licenses CRM
if (!existeinAD)
{
// Delete in AD
// DesEnable in CRM (Delete in CRM)
// Delete Licenses CRM
}
}完整代码：
if (ActionTypePagina == ActionType.Modificacion || ActionTypePagina == ActionType.MisDataM)
{
var bAccionOk = false;
var sPassword = DataPersonales.GenerarPassword();
var dpDataPersonales = fillDataPersonales(sPassword);
var userPortal = fillUserPortal();
var isEmployer = userPortal.IsEmployerRolAorRolB();
var existsEnAD = ADOperations.ExistsUserEnActiveDirectory(dpDataPersonales.sUser);
//var userConsultado = setUserConsultado();
if (userPortal.bIs_Enabled.Value && isEmployer)
{
// 1a) Si RolA, RolB (employer) NO exists en AD ==> Error
// 1b) Si RolA, RolB (employer) exists en AD ==> Enable en CRM (si false, Request New CRM)
if (!existsEnAD)
{
var msg = String.Format("El employer {0:-10} no exists en ActiveDirectory", dpDataPersonales.sUser);
var log = String.Format("User: {0, -15} - Error: {1:-100}", dpDataPersonales.sUser, msg);
LoggerAD.Trace("[AdminDataUsers] - Modificación ERROR. ExistsUserEnActiveDirectory. " + log);
((NsiMaster)Master.Master.Master).ShowNotification(msg, CommunicationType.Error, true);
return;
}
// Enable en CRM (si false, Request New CRM)
var okEnable = AdminManager.EnableUserEnCrm(dpDataPersonales.sUser);
if (!okEnable)
{
var msg = "No es posible habilitar al usuario en CRM. Realice una Request New CRM";
((NsiMaster)Master.Master.Master).ShowNotification(msg, CommunicationType.Error, true);
return;
}
}
if (userPortal.bIs_Enabled.Value && !isEmployer)
{
// 2a) Si NO RolA, RolB (mediador) NO exists en AD ==> Alta en AD y Enable en CRM (si false, Request New CRM)
if (!existsEnAD)
{
var permisoOK_AltaAD = (UserLogado.EsMediadorMA() || UserLogado.IsEmployerRolAorRolB())
&& !userPortal.IsEmployerRolAorRolB();
var okAD = false;
if (permisoOK_AltaAD)
{
var msgEstadoAD = "";
var NombreCompleto = dpDataPersonales.sNombre + " " + dpDataPersonales.sApellido1 + " " + dpDataPersonales.sApellido2;
okAD = ADOperations.AddUserEnActiveDirectory(dpDataPersonales.sUser, dpDataPersonales.sPassword,
dpDataPersonales.sNombre,
dpDataPersonales.sApellido1 + " " + dpDataPersonales.sApellido2
, NombreCompleto
, isEmployer, UserConsultado.sUser_Aire, UserConsultado.sCartera
, dpDataPersonales.sEmail, dpDataPersonales.sTelefonoFijo, dpDataPersonales.sTelefonoMovil
, out msgEstadoAD);
if (!okAD)
{
var log = String.Format("User: {0, -15} - Error: {1:-100}", dpDataPersonales.sUser, msgEstadoAD);
LoggerAD.Trace("[AdminDataUsers] - Modificación ERROR. AddUserEnActiveDirectory. " + log);
}
if (okAD)
{
var log = String.Format("User: {0, -15} - Password: {1:-10}", dpDataPersonales.sUser, PassManager.ShowPassword(dpDataPersonales.sPassword));
LoggerAD.Trace("[AdminDataUsers] - Modificación. AddUserEnActiveDirectory. " + log);
}
}
// Enable en CRM (si false, Request New CRM)
var okEnable = AdminManager.EnableUserEnCrm(dpDataPersonales.sUser);
if (!okEnable)
{
var msg = "No es posible habilitar al usuario en CRM. Realice una Request New CRM";
((NsiMaster)Master.Master.Master).ShowNotification(msg, CommunicationType.Error, true);
return;
}
}
// 2b) Si NO RolA, RolB (mediador) exists en AD ==> Warning (Email) y Enable en CRM (si false, Request New CRM)
if (existsEnAD)
{
// Warning (Email)
AdminManager.EnviarEmailWarning(dpDataPersonales.sUser);
// Enable en CRM (si false, Request New CRM)
var okEnable = AdminManager.EnableUserEnCrm(dpDataPersonales.sUser);
if (!okEnable)
{
var msg = "No es posible habilitar al usuario en CRM. Realice una Request New CRM";
((NsiMaster)Master.Master.Master).ShowNotification(msg, CommunicationType.Error, true);
return;
}
}
}
// DES-Enable
// 1a) Si RolA, RolB (employer) NO exists en AD ==> Error
// 1b) Si RolA, RolB (employer) exists en AD ==> DesEnable en CRM (Delete en CRM) - Delete de Licencias CRM
if (!userPortal.bIs_Enabled.Value && isEmployer)
{
if (!existsEnAD)
{
var msg = String.Format("El employer {0:-10} no exists en ActiveDirectory", dpDataPersonales.sUser);
var log = String.Format("User: {0, -15} - Error: {1:-100}", dpDataPersonales.sUser, msg);
LoggerAD.Trace("[AdminDataUsers] - Modificación ERROR. ExistsUserEnActiveDirectory. " + log);
((NsiMaster)Master.Master.Master).ShowNotification(msg, CommunicationType.Error, true);
return;
}
// DesEnable en CRM (Delete en CRM)
AdminManager.DeshabilitarUserEnCrm(dpDataPersonales.sUser);
// Delete de Licencias CRM
AdminManager.DarDeDeleteComoUserCrmActivo(dpDataPersonales.sUser);
}
if (!userPortal.bIs_Enabled.Value && !isEmployer)
{
// 2a) Si NO RolA, RolB (mediador) NO exists en AD ==> Warning (Email) y DesEnable en CRM (Delete en CRM) - Delete de Licencias CRM
if (!existsEnAD)
{
// Warning (Email)
AdminManager.EnviarEmailWarning(dpDataPersonales.sUser);
// DesEnable en CRM (Delete en CRM)
AdminManager.DeshabilitarUserEnCrm(dpDataPersonales.sUser);
// Delete de Licencias CRM
AdminManager.DarDeDeleteComoUserCrmActivo(dpDataPersonales.sUser);
}
// 2b) Si NO RolA, RolB (mediador) exists en AD ==> Delete de AD y DesEnable en CRM (Delete en CRM) - Delete de Licencias CRM
if (!existsEnAD)
{
// Delete de AD
var res = ADOperations.DeleteUserEnActiveDirectory(dpDataPersonales.sUser);
// DesEnable en CRM (Delete en CRM)
AdminManager.DeshabilitarUserEnCrm(dpDataPersonales.sUser);
// Delete de Licencias CRM
AdminManager.DarDeDeleteComoUserCrmActivo(dpDataPersonales.sUser);
}
}对于重构有什么建议，而不是代码嗅觉，也可能是错误处理？
回答 2
Code Review用户
发布于 2016-07-29 12:57:40
不要使用匈牙利符号不要使用匈牙利符号：bAccionOk，sPassword，。
不要使用非字母数字字符：bIs_Enabled，permisoOK_AltaAD，.
连接字符串很快就变得难以处理。这已经很困难了，例如：var NombreCompleto = dpDataPersonales.sNombre + " " + dpDataPersonales.sApellido1 + " " + dpDataPersonales.sApellido2;。此外，考虑到dpDataPersonales的三个属性，为什么NombreCompleto不是dpDataPersonales的属性？
(而且，如果NombreCompleto只是一个变量，那么它应该是camelCase。)
ADOperations.AddUserEnActiveDirectory(似乎需要一打(！)或者这样的参数。相反，传递一个类，其中每个类都是一个属性；这样就更容易阅读，出错的可能性也更小。而且，考虑到这些参数中有很多是dpDataPersonales的一部分，为什么不简单地传递该类而不是复制它的值呢？
LoggerAD是什么？在我看来，这看起来不像是应该出现在您的WebForms代码后面的东西，但是您没有向我们提供足够的信息。
你的代码部分是英文的，派对是西班牙语的(我想)。你的评论也一样。这是令人困惑的；理想情况下，我建议你坚持英语。
尝试将代码从代码后移到特定的类。具体来说，将UI从业务逻辑中分离出来。
我从您的第二个代码示例可以而且应该是在一个业务逻辑类(或多个)中获得了这样的印象: UI之间的通信仅限于参数类，用于将参数从UI传递到后端，而另一个类则返回结果(成功/错误)和各种消息。
看看您在顶部定义的变量:我认为bAccionOk不在任何地方使用，而sPassword、dpDataPersonales、userPortal等只用于看起来是后端代码的代码。与UI相关的唯一东西是像((NsiMaster)Master.Master.Master).ShowNotification(msg, CommunicationType.Error, true);这样的调用，它的各种参数都可以来自我前面提到的“响应类”。
Code Review用户
发布于 2016-07-29 16:11:10
任何关于重构的建议，不是代码的味道.
如果你想重构你的代码，你应该从代码嗅觉开始。目前看来，至少有四五个不同编码的开发人员编写了这样的代码：
bIs_Enabled // with an underscore
dpDataPersonales // hungarian-notation
isEmployer // wow, this one is correct ;-)
existsinAD // with 's'
existeinAD // without 's'
existsEnAD // ...难怪你很难改造它。几乎不可能读懂它。
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接：
https://codereview.stackexchange.com/questions/136292
复制相关文章
相似问题
腾讯云开发者
Copyright © 2013 - 2026 Tencent Cloud. All Rights Reserved. 腾讯云 版权所有
深圳市腾讯计算机系统有限公司 ICP备案/许可证号：粤B2-20090059 
粤公网安备44030502008569号
腾讯云计算（北京）有限责任公司 京ICP证150476号 | 京ICP备11018762号