{"id":39,"date":"2007-12-19T16:46:23","date_gmt":"2007-12-19T16:46:23","guid":{"rendered":"http:\/\/www.crazygaze.com\/blog\/archives\/39"},"modified":"2010-06-06T10:57:09","modified_gmt":"2010-06-06T10:57:09","slug":"simplifying-code-using-states","status":"publish","type":"post","link":"https:\/\/www.crazygaze.com\/blog\/2007\/12\/19\/simplifying-code-using-states\/","title":{"rendered":"Simplifying code using states"},"content":{"rendered":"<p>Lately I have been refactoring <a href=\"http:\/\/www.crazygaze.com\/blog\/czplayer\">czPlayer<\/a> to add more features and make future maintenance easier.<br \/>\nSome parts of the code are almost 10 years old, when I first started playing with sound coding.<br \/>\nNeedless to say, my code style and quality has improved a lot in 10 years, and it&#8217;s a funny thing to look at code this old. \ud83d\ude42<br \/>\nAs I was refactoring the code, I found a lot of ambiguities and redundancies caused by member variables added over time.<br \/>\nLet me explain&#8230;.<br \/>\nI had a class responsible for MOD processing, and some of the member variables were:<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nclass MODModule\r\n{\r\n\t\/\/ ...\r\n\tbool m_loaded;\r\n\tbool m_playing;\r\n\tbool m_paused;\r\n\tbool m_reachedEnd;\r\n};\r\n<\/pre>\n<p>What&#8217;s the problem with this approach ?<br \/>\nLet&#8217;s see&#8230;<br \/>\nSuppose I want to call methods on the MOD object (e.g: Pause(), Resume(), etc)<br \/>\nI need to make some checks in each method.<\/p>\n<p>To pause&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nif (!m_loaded || m_playing || m_paused)\r\n\treturn SOME_ERROR;\r\nm_pause = true;\r\n\/\/ ... do pause stuff\r\n<\/pre>\n<p>Then to resume&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nif (!m_loaded || !m_paused)\r\n\treturn SOME_ERROR;\r\nm_paused = false;\r\n\/\/ ... do resume stuff\r\n<\/pre>\n<p>When the MOD finishes playing, I needed something like this&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\n\/\/ ... some code that checks for end\r\nm_reachedEnd = true;\r\n\/\/ups... How about m_playing, m_paused?\r\n\/\/We need to change their values, so we don't break code elsewhere\r\nm_playing = false;\r\nm_pause = false;\r\n<\/pre>\n<p>And similar things when playing, stopping, etc.<br \/>\nSee the problems?<\/p>\n<ul>\n<li>We need checks for multiples variables<\/li>\n<li>We need to make sure the variables have valid values in relation to each other<\/li>\n<li>What if we want to add an extra state? For example, &#8220;bool m_fading&#8221;. We would need to examine a lot of code to check how the other variables are used, and hardwire this new one somewhere. Every time you need to add an extra variable, you&#8217;ll need to check more and more code to make sure everything is ok.<\/li>\n<\/ul>\n<p>Now look at the meaning of the variables. They are mostly mutually exclusive.<\/p>\n<ul>\n<li>If it&#8217;s playing, it can&#8217;t be paused and hasn&#8217;t reached the end.<\/li>\n<li>If it&#8217;s paused, it&#8217;s not playing, and hasn&#8217;t reached the end yet.<\/li>\n<li>&#8230; and so on&#8230;.<\/li>\n<\/ul>\n<p>The more data members a class has, the harder it gets to keep the object in a valid state, so, the less data to manage, the better. Less maintenance and less bugs.<\/p>\n<p>One technique I use a lot when trying to simplify code is to look at it with state diagrams whenever possible.<br \/>\nHere it is what happens (kind of) with instances of the MODModule class:<\/p>\n<p><a title=\"mod_states.png\" href=\"https:\/\/www.crazygaze.com\/blog\/wp-content\/uploads\/2007\/12\/mod_states.png\"><img src=\"https:\/\/www.crazygaze.com\/blog\/wp-content\/uploads\/2007\/12\/mod_states.png\" alt=\"mod_states.png\" \/><\/a><\/p>\n<p>I can easily translate it to code with enums.<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nenum MODState\r\n{\r\n    MOD_STATE_NOT_LOADED,\r\n    MOD_STATE_STOPPED,\r\n    MOD_STATE_PLAYING,\r\n    MOD_STATE_PAUSED,\r\n    MOD_STATE_REACHED_END\r\n};\r\n\r\nclass MODModule\r\n{\r\n    \/\/ ...\r\n    MODState m_state; \/\/ Now we have only one variable\r\n};\r\n<\/pre>\n<p>This approach is a lot easier to maintain, and less error prone.<br \/>\nNow the code in most functions become something like this:<\/p>\n<p>To play&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nif (m_state!=MOD_STATE_STOPPED)\r\n    return SOME_ERROR;\r\nm_state = MOD_STATE_PLAYING;\r\n\/\/ ... do play stuff\r\n<\/pre>\n<p>To pause&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nif (m_state!=MOD_STATE_PLAYING)\r\n    return SOME_ERROR;\r\nm_state = MOD_STATE_PAUSED;\r\n\/\/ ... do pause stuff\r\n<\/pre>\n<p>To resume&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\nif (m_state!=MOD_STATE_PAUSED)\r\n    return SOME_ERROR;\r\nm_state = MOD_STATE_PLAYING;\r\n\/\/ ... do resume stuff\r\n<\/pre>\n<p>To stop&#8230;<\/p>\n<pre class=\"brush: cpp; title: ; notranslate\" title=\"\">\r\n\/\/ We cannot stop if it's not even loaded yet, or it's already stopped\r\nif (m_state==MOD_STATE_NOT_LOADED || m_state==MOD_STATE_STOPPED)\r\n    return SOME_ERROR;\r\nm_state = MOD_STATE_STOPPED;\r\n\/\/ ... do stop stuff\r\n<\/pre>\n<p>The code may look similar, but it&#8217;s a lot less error prone, easier to follow and maintain.<br \/>\nLooking at the State Diagram, I could easily squeeze in a &#8220;Fading&#8221; state and understand the conditions to enter or leave that state.<\/p>\n<p>As a bonus, using enums will enforce more rules and help you out with errors the compiler might catch. Always prefer compile-time errors over run-time errors. \ud83d\ude09<\/p>\n","protected":false},"excerpt":{"rendered":"<p>Lately I have been refactoring czPlayer to add more features and make future maintenance easier. Some parts of the code are almost 10 years old, when I first started playing with sound coding. Needless to say, my code style and quality has improved a lot in 10 years, and it&#8217;s a funny thing to look [&hellip;]<\/p>\n","protected":false},"author":3,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_mi_skip_tracking":false,"spay_email":"","jetpack_publicize_message":"","jetpack_is_tweetstorm":false,"jetpack_publicize_feature_enabled":true},"categories":[3],"tags":[12,13,6],"jetpack_featured_media_url":"","jetpack_publicize_connections":[],"jetpack_sharing_enabled":true,"jetpack_shortlink":"https:\/\/wp.me\/p7jpe0-D","_links":{"self":[{"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/posts\/39"}],"collection":[{"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/users\/3"}],"replies":[{"embeddable":true,"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/comments?post=39"}],"version-history":[{"count":0,"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/posts\/39\/revisions"}],"wp:attachment":[{"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/media?parent=39"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/categories?post=39"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.crazygaze.com\/blog\/wp-json\/wp\/v2\/tags?post=39"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}