Refactoring of a java class

Asked

Viewed 388 times

9

I have a Java class that contains 1756 lines (obviously not all code, has blank lines, many comments and some commented code in case it is needed in the future)

I’m implementing the structure MVC and this class belongs to the control.

This class is responsible for:

  • manage customers( communication with customers, save who is connected, send information to them, know who left etc)
  • retrieve data from a web service and a database (synchronously)
  • Process this data and send this information to customers
  • Store important data information in the system database

Class structure ServidorService:

//variaveis
 + 8 variaveis 
 + de 12 HashMap //quase todos os dados estão relacionados key->value ex: clientex->outputx

//classes

img1 img2 img3

Problem

The class is getting really big for me, but thinking about someone’s future need to change some function I think it will be difficult for her to maintain the code.

What’s the best way to refactor it?

Main doubts:

  • I create another type class ServidorService_2 and I try to divide?

  • I tried to create another class to support the main one (Servorservice) but I was having problems accessing the variables, what is the best way to make the other class have access to the variables? through gets?

  • 6

    At the time you listed 4 responsibilities of this class, you yourself identified at least 4 new classes. For you to like your code, each class must have a single responsibility. Do not ServidorService_2 and ServidorService_3 because it will not improve at all your code (maybe it gets worse). To have access to variables, yes, it is through gets. But you should not publish "variables" but "attributes" of your class (the technique is the same but the concept is different and helps to see if the code is ok). I’m not gonna put that as an answer because I think it’s off-topic to the O.R..

  • 3

    No matter what you do, do not create Servorservice_2!! I believe that much of this code should be in Controller, another part of DAO with local BD, another in DAO for webservice (regardless of the name you prefer to give to that layer)

  • 2

    thanks @Math as for Servorservice_2 was just a stupid example but I didn’t even know what other name I could give now with the help of @Caffé and with the reply of @Cold I already have an idea how to do... For me the "Servor_service" is my Controller and in it whenever I need some data, be it the webservice or BD just call the method that is in other classes: Ex: new ProdutoDao().guardar(produto); or listProdutos=getProdutosWebService(); this is what you are referring to?

  • 1

    @jsantos1991 good that already has a light! But in my opinion, I would do the part of access to the webservices outside the controller, in a specific DAO, I even have a name for that layer, but I do not know if it is consensus, I call Resource. Maybe I’m talking nonsense, but this is how I do and I think it meets the principles of OO.

  • 1

    @Caffé Thanks only one doubt, when it refers to "variables" and yes "attributes" it means that it is wrong to say "variables" right? because the correct name is "attributes" just to clarify what you said.

  • 1

    @Math yes as you said and very well I only have a light, so I’m never quite sure if I’m doing it right or not, but as for that part I think it’s right and fortunately, in this case as it has no graphical interface because I think it’s when I can destroy all the mvc [but this part is already [OFF-TOPIC]]

  • 1

    @jsantos1991 It’s not the right name, the important thing is to understand that a class publishes the data it wants to expose, its characteristics and state, which can be a little deeper than simply increasing the visibility of a variable. This subject is extensive, yields useful discussions, but unsuitable for this space.

  • 1

    @Caffé gave to clarify a little more, but I still could not understand how I liked, if you can send a link|keywords that talk about it [EN or PT] I was grateful.

  • 1

    @jsantos1991 there is no right or wrong, there is appropriate or inappropriate. The code should be appropriate for your needs, of course it is often difficult to see in the medium or long term what is most appropriate for you. If your software is long-lived you will probably have to refactor it a few more times, and with experience you will suffer less in subsequent projects. To begin with, try never to escape the most basic OO principles, such as single responsibility (high cohesion) and low coupling.

  • 2

    @jsantos1991 These are Object Orientation concepts. This link that I found now looks good: https://www.caelum.com.br/apostila-java-orientacao-objetos/ This section in particular should give a notion of the conceptual difference between publishing a variable or attribute (who guarantees the business irrigation, as validation of the set values?): http://www.caelum.com.br/apostila-java-orientationObject/modifiersaccess-e-attribute-class/#6-1-controlling-the-access

  • 5

    (...)e algum código comentado para o caso de ser necessário no futuro(...) That’s more toxic than uranium. Get rid of that code. If that code ever becomes necessary, you can retrieve it from the repository. If this happens, play the lottery too, because a lifetime of experience with it says that code commented back to be useful is something that occurs once every blue moon. And if you don’t have version control, stop the project until you start using a.

  • 1

    @Math thanks, at first it’s really complicated to know what is appropriate or not...

  • 1

    @Caffé thanks for the Links...

  • 2

    @Renan thanks for the two tips, the commented code that I will remove and the version control, that I am not actually using (I have done by hand), but this project is in the final phase soon a next without a doubt that I will use.

Show 9 more comments

3 answers

4


For the responsibilities you mentioned I suggested the following (class names are just suggestions, you can put to your liking):

1 - Create classes with the implementation of specific methods according to responsibilities

GestaoClientes.java : With the implementation of methods for customer management (Responsibility 1 and 3);

AcessoBD.java : With the implementation of all methods of access and interaction with the Database (Responsibility 1b and 4);

AcessoWebServices.java : With the implementation of all methods that interact with WebServices(Liability 1a).

Note: Responsibility 1a is "fetch data from a web service" and the liability 1b is "to a database".

2 - Keep server-specific and related methods in class ServidorService and variables (those to be visible or modified by all methods).

If all variables remain in the class ServidorService then I believe that the methods in the classes I suggested can be static. And you will only invoke the methods when you need them.

I noticed that you also have an inner class and I still don’t understand her idea there, but I don’t know what to suggest about her.

In this way I believe that you have already greatly decreased the amounts of lines and facilitate the maintenance of the code, because the other technician would know where to look specifically according to the objective.

3

The MVC structure indicates you use Model where you will create class referring to your entities within it we have the class attributes and the get’s and set’s plus the toString() only this, if you are using database you can create classes called DAO(Data Access Object) each entity has its own DAO, The Controller is for business rule or whether each entity has its Controler, and the View is for displaying information.

You will separate everything in example package:

br.nomeProjeto.model
br.nomeProjeto.controler
br.nomeProjeto.view
br.nomeProjeto.dao

Good practices today say that a very commented code is not well elaborated if necessary create interfaces and implement their methods. I hope I’ve helped.

  • I hope I didn’t confuse you, but your answer doesn’t answer my question, my doubt is NOT about the structure of the MVC, I spoke in MVC and just to say that this class belongs to the controller, my doubt is about REFACTORING...

  • @jsantos1991 as you said that this class is already in the Controller, I suggest you follow the principle of sole responsibility and create a Controller for each entity, or for each relevant entity, that is, some may be left out if access to them can be made by a dependency of another class. Also, some of these methods do not belong to the Controller, create an appropriate layer for them.

2

I think it would be interesting for you to abstract methods that are not directly linked to the function of your class, such as utilitarian methods, but answering your question:

  1. If your methods are linked/related to what your class proposes I see no reason to create a new class, of course each has a different view of the subject;
  2. It is recommended to always access attributes from other classes using getters and setters, low coupling, and high cohesion.

Browser other questions tagged

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