1

I'm designing an app which use a Knowledge Base filled with rules. Depending on the context some rules will be applicable and others will not.

  • I need to get first the list of the applicable rules and optionally later the list of non applicable rules.
  • The system doesn't need both at the same time and I want to avoid keeping them in memory while not needed.
  • I also want to search through my list of rules only one time for optimization purpose.

I thought of having a KnowledgeBase object containing a list of all rules with two empty attributes for applicable and non applicable rules at the creation of the object. When created, I only need the full list of rules without context being necessary.

KnowledgeBase class

I don't give more details about the context because id doesn't matter.

function applicableRules(context) {
    if !isset(applicableRules) {
       sortRules(context);
    } 
    return applicableRules;
}

function inapplicableRules(context) {
    if !isset(applicableRules) {
        sortRules(context);
    } 
    return inapplicableRules;
}

function sortRules(context) {
    foreach rules {
        if rule.isApplicable(context) {
            applicableRules.add(rule);
        } else {
            nonApplicableRules.add(rule);
        }
    }
    unset(rules);
}

function rules() { //Return all the rules
    if !isset(rules) {
        return applicableRules+inapplicableRules;
    } else {
        return rules;
    }
}

But doing this means I have 3 attributes in my object that will never be instantiated at the same time (rules VS applicableRules + nonApplicableRules)

The programming language doesn't matters, I'm just trying to know if the concept is okay

So is it a good way to do things or is this just awful ?

  • The main problem with this is that it violates the DRY principle because you're needing to do multiple repeated checks for !isset(rules) and then switch behaviour depending up on whether it's sorted or not - there's just a bunch of extra unnecessary complexity there. I would consider moving the sort logic away from this class altogether so that KnowledgeBase is only ever dealing with rules after they have already been sorted. It seems that the sorted/not-sorted status depends upon the context so it seems like you'd be better off with one KnowledgeBase instance per context – Ben Cottrell Apr 18 at 17:03
2

Any time we have instance fields that are unused in one scenario or another, this suggests that we are conflating separate objects into one.

Ideally, all the instance fields of a single object have the same lifetime — namely that of the object itself.

So when we have instance fields that have differentiated lifetimes, this indicates that logically we have several different types.


Your context parameter is problematic, since it is only consulted the first time it is passed, and otherwise goes unused (instead deferring to the original).  For naive consuming clients, this means unexpected results when they use another context.  (We should provide interfaces and abstractions for naive clients, so that consumption leads to falling into the pit of success.)

(Again, there appears to be a lifetime issue here, and this time it suggests that the context parameter belongs in the constructor.)


Sure, you can program like this but better practices are available if you want to write a larger code base and/or maintainable code.

We don't know why you are concerned with memory consumption or optimization at this point in your project, but the common wisdom is that your code maintainability is more important and that performance should be a lessor concern until you actually run into measurable performance problems.  Almost always, performance problems appear in unexpected places, so solving them before measuring is generally not worthwhile.

(I would also assert that in the long run, it would be easier to reason over and improve the performance of code that avoids unexpected behaviors like mixing different lifetimes in a single object.)

1

TBH this is just awful. The additional slots are used as a kind of context-specific cache, but this cache is not invalidated when another context is used. Unsetting the rules slot is not really necessary (saved memory is probably negligible) but could be dangerous.

Much safer would be to have an explicit cache keyed by (knowledgeBase, context) storing the applicable and inapplicable rules for each pair.

1

Its fine. You have a thing called a Knowledge Base. It needs two types of rules. These aren't unused attributes, they are attributes that aren't used at the same time.

In your specific case, since you never need both sets of rules loaded at once, you can probably get away with a single var to hold both. You also mention memory optimization. I would not worry about that unless you know it will be an issue.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.