{"id":1081,"date":"2010-08-21T11:56:13","date_gmt":"2010-08-21T09:56:13","guid":{"rendered":"http:\/\/www.limilabs.com\/blog\/?p=1081"},"modified":"2012-02-15T15:15:02","modified_gmt":"2012-02-15T13:15:02","slug":"better-public-api-getaccountstat","status":"publish","type":"post","link":"https:\/\/www.limilabs.com\/blog\/better-public-api-getaccountstat","title":{"rendered":"Better public API: GetAccountStat"},"content":{"rendered":"<p>  Here&#8217;s some code I recently found in <a href=\"\/mail\">Mail.dll<\/a> and decided to refactor.<\/p>\n<pre class=\"brush: csharp; title: ; notranslate\" title=\"\">\r\n\/\/ Before:\r\n\r\nusing(Pop3 client = new Pop3())\r\n{\r\n    client.GetAccountStat();\r\n    Console.WriteLine(\r\n        &quot;Inbox has {0} emails.&quot;,\r\n        client.MessageCount);\r\n}\r\n<\/pre>\n<p>There are several things <strong>wrong<\/strong> with this code.<\/p>\n<ul>\n<li>The method is called <strong>Get&#8230;<\/strong> but it does not get anything, <strong>it changes the internal state<\/strong> of the object.<\/li>\n<li>Message count is stored in Pop3 object:\n<ul>\n<li>If the user of your API connects later to a different server you need to remember to reset this variable.<\/li>\n<li>If the user of your API forgets to call GetAccountStat, message count is undefined (Should it be null?)<\/li>\n<\/ul>\n<\/li>\n<\/ul>\n<p>It&#8217;s really hard to say that this is a friendly API, as it requires the user to <strong>perform actions in specific order<\/strong> (call GetAccountStat before accessing message count).<\/p>\n<p>Another problem is that GetAccountStat method is <strong>responsible for parsing the server response<\/strong>. It&#8217;s not necessary a bad thing, but if you have hundreds such methods then Pop3 class gets bloated with hard-to-test parsing logic.<\/p>\n<p>Now lets take a look at the <strong>After<\/strong> code:<\/p>\n<pre class=\"brush: csharp; title: ; notranslate\" title=\"\">\r\n\/\/ After:\r\n\r\nusing(Pop3 client = new Pop3())\r\n{\r\n    AccountStats stats = client.GetAccountStat();\r\n    Console.WriteLine(\r\n        &quot;Inbox has {0} emails.&quot;,\r\n        stats.MessageCount);\r\n\r\n}\r\n<\/pre>\n<p>Here we can see a <strong>good API<\/strong>:<\/p>\n<ul>\n<li>Method is called Get&#8230;. and it actually gets something.<\/li>\n<li><strong>No specific call order is required<\/strong>, you simply call one method and act on the result.<\/li>\n<li><strong>Parsing logic was moved<\/strong> to the AccountStats class.<\/li>\n<\/ul>\n<p>This is not seen here but AccountStats method has a static Parse method&#8230;and look how easy is to write unit test for it&#8217;s behavior:<\/p>\n<pre class=\"brush: csharp; title: ; notranslate\" title=\"\">\r\n&#x5B;Test]\r\npublic void Parse_MessageCountAndMailboxSize_AreFilled()\r\n{\r\n    AccountStats stats = AccountStats.Parse(&quot;2 373766&quot;);\r\n    Assert.AreEqual(2, stats.MessageCount);\r\n    Assert.AreEqual(373766, stats.MailboxSize);\r\n}\r\n<\/pre>\n<p>Note also that actually <strong>we have NOT introduced a breaking change<\/strong> to our public API. Following code still works:<\/p>\n<pre class=\"brush: csharp; title: ; notranslate\" title=\"\">\r\nusing(Pop3 client = new Pop3())\r\n{\r\n    client.GetAccountStat();\r\n    Console.WriteLine(\r\n        &quot;Inbox has {0} emails.&quot;,\r\n        client.MessageCount);\r\n}\r\n<\/pre>\n<p>You&#8217;ll get 2 obsolete warnings:<\/p>\n<p><code>warning CS0618: 'Limilabs.Client.POP3.Pop3.MessageCount' is obsolete: 'Please use the return value of GetAccountStat method instead.'<\/code><\/p>\n<p><code>warning CS0618: 'Limilabs.Client.POP3.Pop3.MailboxSize' is obsolete: 'Please use the return value of GetAccountStat method instead.'<\/code><\/p>\n<p>As we marked MessageCount and MailboxSize with <strong>[obsolete]<\/strong> attribute, but that&#8217;s it!<\/p>\n","protected":false},"excerpt":{"rendered":"<p>Here&#8217;s some code I recently found in Mail.dll and decided to refactor. \/\/ Before: using(Pop3 client = new Pop3()) { client.GetAccountStat(); Console.WriteLine( &quot;Inbox has {0} emails.&quot;, client.MessageCount); } There are several things wrong with this code. The method is called Get&#8230; but it does not get anything, it changes the internal state of the object. [&hellip;]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"closed","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[6],"tags":[17],"class_list":["post-1081","post","type-post","status-publish","format-standard","hentry","category-programming","tag-clean-code"],"aioseo_notices":[],"_links":{"self":[{"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/posts\/1081"}],"collection":[{"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/comments?post=1081"}],"version-history":[{"count":3,"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/posts\/1081\/revisions"}],"predecessor-version":[{"id":2498,"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/posts\/1081\/revisions\/2498"}],"wp:attachment":[{"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/media?parent=1081"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/categories?post=1081"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.limilabs.com\/blog\/wp-json\/wp\/v2\/tags?post=1081"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}