Zombie code found

[Leo-Nogismus]

While I was going through the reduce() function in the mdl_controller.cpp (both in PrimaryMDLOverlay::reduce(...) and SecondaryMDLOverlay(...)), I found some things that are probably just zombie code flying around. Maybe we want to remove them at some point. Listing everything I find here. This can be fixed/ extended at a later date.

1. chaining_allowed is (I believe) always true in line 166 This comes from chaining_allowed = (c_s >= WEAK_REQUIREMENT_ENABLED); on line 143. With WEAK_REQUIREMENT_ENABLED = 3 and NO_REQUIREMENT = 4 in ChainingStatus this means that in the case of NO_REQUIREMENT (as is the case in line 160) c_s is always true. We should, thus, be able to remove lines 160-162 without any impact on the code.

2. Line 163 is not necessary either. If c_s is either WEAK_REQUIREMENT_DISABLED, STRONG_REQUIREMENT_NO_WEAK_REQUIREMENT, or NO_REQUIREMENT it follows that wr_enabled in line 141 is false (see method retrieve_imdl_fwd for more information). Thus, since line 141 states f_imdl->get_reference(0)->code(I_HLP_WEAK_REQUIREMENT_ENABLED) = Atom::Boolean(wr_enabled); with wr_enabled = false it follows that we do not need line 163, which states f_imdl->get_reference(0)->code(I_HLP_WEAK_REQUIREMENT_ENABLED) = Atom::Boolean(false);, which is exactly the same thing.

3. Line 254: chaining_allowed is not used anywhere later on and thus can be safely removed.

4. Same as 2., but this time on line 262.

I think most of this comes from the removed

#if 0 // JTNote: We set ground = NULL above, so (ground != NULL) is never true.
    if (ground != NULL) { // an imdl triggered the reduction of the cache.

      r_p.weak_requirements_.controllers.insert(req_controller);
      r_p.weak_requirements_.f_imdl = ground;
      r_p.weak_requirements_.chaining_was_allowed = true;
      return WEAK_REQUIREMENT_ENABLED;
    }
#endif

or

#if 0 // JTNote: We set ground = NULL above, so (ground != NULL) is never true.
      if (ground != NULL) { // an imdl triggered the reduction of the cache.

        requirements_.CS_.leave();
        float32 confidence = ground->get_pred()->get_target()->get_cfd();
        if (confidence > negative_cfd) {

          r = WEAK_REQUIREMENT_ENABLED;
          r_p.weak_requirements_.controllers.insert(req_controller);
          r_p.weak_requirements_.f_imdl = ground;
          r_p.weak_requirements_.chaining_was_allowed = true;
          wr_enabled = true;
        }
        return r;
      }
#endif

that removes the need for those checks.

[jefft0]

Nice catch. If I had a meeting with Eric and could ask one question, it would be about the design of chaining_allowed and the related I_HLP_WEAK_REQUIREMENT_ENABLED. I agree that you found dead code. The code that I commented out was probably planned for a future use. This is part of the reason that I think we should have unit tests. Then we could remove apparently dead code and make sure that the unit tests still pass.