2

I use java language to develop android application, after static analysis, the tool warns about high efferent coupling.

What is the reason of this warning? Is it about AComModel which is extended by other class?

There are 4 classes below which the tool warns us about them. I hope I asked logical question, but I really don't understand what is the problem?

public class AComModel {    
    public String GetComBusinessKey() {
        return "none";
    }
}


@JsonIgnoreProperties(ignoreUnknown = true)
public class StepOneCom extends AComModel {

    public View vi;
    public ArrayList<SummaryViewModel> lstSmr;

    @Override
    public String GetComBusinessKey() {
        return "shvi";
    }

}


@JsonIgnoreProperties(ignoreUnknown = true)
public class StepTwoCom extends AComModel {

    public ArrayList<AccViewModel> lstAcc;
    public SummaryViewModel c1;


    @Override
    public String GetComBusinessKey() {
        return "shvi";
    }

}


@JsonIgnoreProperties(ignoreUnknown = true)
public class StepThreeStepTwoCom extends AComModel {

    public StepFourCom stepFourCom;

    @Override
    public String GetComBusinessKey() {
        return "cashadvi";
    }
}

1 Answer 1

3

Without seeing full code in your project, it is impossible to tell with 100% certainty, but given that tool was performing static code analysis, this warning indicates either of:

  • StepOneCom doesn't use specific methods and fields of either of classes View, ArrayList, SummaryViewModel
  • StepTwoCom doesn't use specific methods and fields of either of classes AccViewModel, ArrayList, SummaryViewModel
  • StepThreeCom doesn't use specific methods and fields of class StepFourCom

or some subset of what is listed above, or even all of the above together.

Probably simplest way to verify exactly what it is complaining about would be to replace (where it doesn't break compilation) specialized classes mentioned above with most general Object, and check whether warnings disappear.

This is based on explanation of efferent coupling given in Wikipedia:

a metric in software development. It measures the number of data types a class knows about.

This includes inheritance, interface implementation, parameter types, variable types, and exceptions.

A large efferent coupling can indicate that a class is unfocused. It may also indicate brittleness, since it depends on the stability of all the types to which it is coupled.

In simpler terms, if you are able to replace some specialized type with a standard one, like Object, without breaking compilation, this means that there is no real need for your class to be aware of that specialized type, and that introducing this "useless knowledge" you only make the code harder to maintain.

Reader of your code would have to break their mind trying to guess why did you involve View, ArrayList, SummaryViewModel, AccViewModel, StepFourCom where compiler apparently tells them that plain standard Object would do just as good.


While we're at it, I would like to also point out that the way presented in the snippet, code appear to make a probably even worse violation of the principles suggested in basic language tutorial:

  • Use the most restrictive access level that makes sense for a particular member. Use private unless you have a good reason not to.
  • Avoid public fields except for constants... Public fields tend to link you to a particular implementation and limit your flexibility in changing your code.

Unless you are 200% sure that you need fields to be public and can give a bullet proof justification for this, think of public access modifiers in code like this as a source of your worst future headaches and nightmares:

public View vi;
public ArrayList<SummaryViewModel> lstSmr;

Guidance given in tutorial is there for a reason, it is based on a painful experience of multiple programmers who tried this way and discovered that it's a dead end.

A less blatant, but still quite slippery part of the design in your code snippet is unjustified use of inheritance. Inheritance puts certain extra obligations and risks on classes using it, and you better be sure that it's exactly what you need.

I mention this because the way how code is written in your code snippet, it can be equally as well do if AComModel to be an interface instead of class and just declare GetComBusinessKey instead of providing default implementation, so that StepOneCom, StepTwoCom, StepThreeCom would implement that interface, instead of extending a class.

3
  • What you say is only true for a properly encapsulated class, this is not the case here as all fields are public (i.e. an unrelated piece of code might rely on these specific types). So the first step before switching to the most general type that still works would be to make the fields private, and to provide accessors if necessary.
    – amon
    Commented Feb 18, 2014 at 15:10
  • @amon that's why I pointed that tool did static code analysis - as far as I can tell, it simply checked all the code in the project and discovered that none refers to specific methods / fields. As for private fields, I just edited the answer to address that, it was too hard to put two (three to be precise) complaints into a single shot
    – gnat
    Commented Feb 18, 2014 at 15:19
  • 1
    @gnat thank you very much for your detailed information
    – talhakosen
    Commented Feb 19, 2014 at 9:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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