What can be done to improve this code?

Asked

Viewed 106 times

-2

class Curso {
    public List<Disciplina> disciplinas;
}

class Aluno {
    public List<Disciplina> obrigatorias;
    public List<Disciplina> optativas;


    public matricula(){
        ...
    }
}

class Disciplina {
    List<Curso> pertencentes;

    public Disciplina(Curso curso) {
        this.pertence = curso;  
    }

    public getCurso(){
        return this.curso;
    }

    public setCurso(Curso curso){
        this.curso = curso;
    }
}

My OO teacher gave me these classes to analyze. I need to find the maximum error or bad practices. From what I can tell, it’s all right, but I was wondering if there’s something wrong with the code? He mentioned having low coupling and responsibilities.

  • Did the answer solve your question? Do you think you can accept it? See [tour] if you don’t know how you do it. This would help a lot to indicate that the solution was useful for you. You can also vote on any question or answer you find useful on the entire site (when you have 15 points).

1 answer

4

It is very complicated to talk about good practices, you have a lot of opinion about it. Nor can you talk much without knowing the requirements, the concrete case. There is no good practice in an abstract case, it is only good practice to apply within what you need. Artificial examples do not serve to teach good practice, only to teach cake recipe that people will mistakenly apply in real cases.

An example that many people will say is good practice is to create getters and setters that you created. This is bad practice. Do you know why? Because you did it without understanding why. You might even be right, as I said before, it depends on the specific case, but as someone did before and said it’s good, it’s bad practice. You can start further studying the subject at Getters and Setters Methods.

Doesn’t have a builder, almost always, but not always, it is good practice to have one. And in most cases the Setter is inappropriate.

Are you sure it is appropriate to keep the fields which are lists as public or internal to the package as you did? Both stated so explicitly and implicitly. That’s abstraction leak. What if it is more appropriate to use in another way forward? You can also read Programming for interface and not for implementation, why?.

I don’t know if Aluno should have a method called matricula(). This may be an improper coupling, but again, no requirement has no way of knowing. See more on What are the concepts of cohesion and coupling?. It can also be about having undue responsibility, see What is and how to use SRP?.

You can’t even go much further because the code is half done, it doesn’t use the array created, uses some fields (and not attributes as people call) that have not been declared in the class.

Object-oriented programming is difficult and without clear requirements is impossible even for the more experienced. Even with all this there are not always definitive answers and it is naive to find it and can become rule poo :)

Browser other questions tagged

You are not signed in. Login or sign up in order to post.