{"id":393855,"date":"2024-06-29T11:15:59","date_gmt":"2024-06-29T11:15:59","guid":{"rendered":"http:\/\/savepearlharbor.com\/?p=393855"},"modified":"-0001-11-30T00:00:00","modified_gmt":"-0001-11-29T21:00:00","slug":"","status":"publish","type":"post","link":"https:\/\/savepearlharbor.com\/?p=393855","title":{"rendered":"A variety of errors in C# code by the example of CMS DotNetNuke: 40 questions about the quality<\/span>"},"content":{"rendered":"
<\/div>\n
\n
\n
\n
\n

\"0890_DNN\/image1.png\"<\/p>\n

<\/p>\n

Today, we discuss C# code quality and a variety of errors by the example of CMS DotNetNuke. We’re going to dig into its source code. You’re going to need a cup of coffee…<\/p>\n

<\/a> <\/p>\n

DotNetNuke<\/h2>\n

<\/p>\n

DotNetNuke is an open-source content management system (CMS) written mainly in C#. The source code is available on GitHub<\/a>. The project is part of the .NET Foundation<\/a>.<\/p>\n

<\/p>\n

\"0890_DNN\/image2.png\"<\/p>\n

<\/p>\n

The project has its website<\/a>, Twitter<\/a>, YouTube channel<\/a>. <\/p>\n

<\/p>\n

However, I still don’t understand the project status. The GitHub repository is updated from time to time. They have new releases. Though, it’s been a while since they post something on Twitter or YouTube channel.<\/p>\n

<\/p>\n

At the same time, they have a community website<\/a> where you can find information about some events.<\/p>\n

<\/p>\n

Anyway, we are interested in the code especially. The code and its quality.<\/p>\n

<\/p>\n

By the way, the project web page<\/a> (see a screenshot below) shows that the developers use the NDepend<\/a> static analyzer to monitor code quality.<\/p>\n

<\/p>\n

\"0890_DNN\/image3.png\"<\/p>\n

<\/p>\n

I don’t know how the project developers configured the analyzer, whether warnings are handled, and so on. But I’d like to remind you that it’s better to use static analysis tools regularly in your development process. You can find lots of articles on this topic \u2013 visit our blog<\/a> to read some.<\/p>\n

<\/p>\n

About the check<\/h2>\n

<\/p>\n

To check the project, I used the source code from GitHub<\/a> from the 22nd of October 2021. Take into account that we published \/ you read this article after a while. The code may be different by now.<\/p>\n

<\/p>\n

I use PVS-Studio<\/a> 7.15 to perform the analysis. Want to try the analyzer on your project? Click here<\/a> to open the page with all the necessary steps. Have any questions? Don’t understand something? Feel free to contact us<\/a>.<\/p>\n

<\/p>\n

Today, I’d like to start with one of the new features of PVS-Studio 7.15 \u2013 the best warnings list. The feature is brand-new, and we will enhance it in the future. However, you can (and should) use it right now.<\/p>\n

<\/p>\n

Best warnings<\/h2>\n

<\/p>\n

Let’s say you decide to try a static analyzer on your project. You downloaded it, analyzed the project, and\u2026 got a bunch of warnings. Tens, hundreds, thousands, maybe even tens of thousands. Wow, «cool»\u2026 It would be great to magically select, for example, the Top 10 most interesting warnings. Enough to look at and think: «Yeah, that code is rubbish, definitely!». Well, now PVS-Studio has such a mechanism. It is called best warnings<\/a>.<\/p>\n

<\/p>\n

So far, you can use the feature only in the PVS-Studio plugin for Visual Studio. But we plan to add the best warnings to other IDE plugins later. With the best warnings mechanism, the analyzer selects the most interesting and plausible warnings from the log.<\/p>\n

<\/p>\n

Ready to see the best warnings list for the DNN project?<\/p>\n

<\/p>\n

Best warnings. Issue 1<\/strong><\/p>\n

<\/p>\n

public string NavigateURL(int tabID,                            bool isSuperTab,                            IPortalSettings settings,                            ....) {   ....   if (isSuperTab)   {     url += \"&portalid=\" + settings.PortalId;   }    TabInfo tab = null;   if (settings != null)   {     tab = TabController.Instance.GetTab(tabID,              isSuperTab ? Null.NullInteger : settings.PortalId, false);   }   .... }<\/code><\/pre>\n
  <\/p>\n
The PVS-Studio warning: V3095<\/a> The ‘settings’ object was used before it was verified against null. Check lines: 190, 195. DotNetNuke.Library NavigationManager.cs 190<\/p>\n
  <\/p>\n
I wonder why at first we access the settings.PortalId<\/em> instance property, and then we check settings<\/em> for null<\/em> inequality. Thus, if settings<\/em> \u2013 null<\/em> and isSuperTab<\/em> \u2013 true<\/em>, we get NullReferenceException<\/em>.<\/p>\n
  <\/p>\n
Surprisingly, this code fragment has a second contract that links isSuperTab<\/em> and settings<\/em> parameters \u2013 the ternary operator: isSuperTab? Null.NullInteger: settings.PortalId<\/em>. Note that in this case, unlike if<\/em>, settings.PortalId<\/em> is used when isSuperTab<\/em> is false<\/em>.<\/p>\n
  <\/p>\n
If isSuperTab<\/em> is true<\/em>, the settings.PortalId<\/em> value is not processed. You may think that it’s just an implicit contract, and everything is fine.<\/p>\n
  <\/p>\n
Nope.<\/p>\n
  <\/p>\n
The code must be easy to read and understandable \u2013 you don’t have to think like Sherlock. If you intend to create this contract, write it explicitly in the code. Thus, the developers, the static analyzer, and you will not be confused. \ud83d\ude09<\/p>\n
  <\/p>\n
Best warnings. Issue 2<\/strong><\/p>\n
  <\/p>\n
private static string GetTableName(Type objType) {   string tableName = string.Empty;    \/\/ If no attrubute then use Type Name   if (string.IsNullOrEmpty(tableName))   {     tableName = objType.Name;     if (tableName.EndsWith(\"Info\"))     {       \/\/ Remove Info ending       tableName.Replace(\"Info\", string.Empty);     }   }   .... }<\/code><\/pre>\n
  <\/p>\n
The PVS-Studio warning: V3010<\/a> The return value of function ‘Replace’ is required to be utilized. DotNetNuke.Library CBO.cs 1038<\/p>\n
  <\/p>\n
Here we have several curious cases:<\/p>\n
  <\/p>\n