C
C
Chvalov2016-03-11 10:39:46
Java
Chvalov, 2016-03-11 10:39:46

Is the same piece of code in different classes acceptable or noob?

This piece of code is used in two different classes (MainActivity.java and Response.java):

// Подсчет тока по каждому двигателю
    private float getCurrent(int engine){
        float phasesA = calculatePhaseForEngine(0, MassCurrentsPhases, engine);
        float phasesB = calculatePhaseForEngine(1, MassCurrentsPhases, engine);
        float phasesC = calculatePhaseForEngine(2, MassCurrentsPhases, engine);

        return (phasesA + phasesB + phasesC) / 3;
    }
    // Подсчетываем ток по выбраному двигателю
    private static float calculatePhaseForEngine(int i, float[] MassCurrentsPhases, int currentEng) {
        return MassCurrentsPhases[3 * currentEng + i];
    }

There are also waters such sections of code in one class (Response) are used in a loop:
switch (data.length) {
            case 96:    // Работа со статусом
                for (int i = 0, j = 0; i < 27; i++, j += 2) {
                    FirstByteTmp = (0x000000FF & ((int) data[21 + j]));
                    SecondByteTmp = (0x000000FF & ((int) data[20 + j]));
                    ByteTmp = (char) (FirstByteTmp << 8 | SecondByteTmp);
                    MassCurrentsPhases[i] = round((float) ByteTmp / 100, 2);
                }
                // .................................
                //..................................
case 71:    // Ковыряем ответ с длиной в 71 значение
                if(data[2] == 0){   // Смотрим 2 значение и если там 0 выполняем код ниже
                    for (int i = 0, j = 0; i < 30; i++, j += 2) {   // Записываем даные в массив
                        FirstByteTmp = (0x000000FF & ((int) data[5 + j]));
                        SecondByteTmp = (0x000000FF & ((int) data[4 + j]));
                        ByteTmp = (char) (FirstByteTmp << 8 | SecondByteTmp);
                       // .................................
                       //..................................
                       } else if(data[2] == 1){ // Смотрим 2 значение и если там 1 выполняем код ниже
                    for (int i = 0, j = 0; i < 30; i++, j += 2) {   // Записываем даные в массив
                        FirstByteTmp = (0x000000FF & ((int) data[5 + j]));
                        SecondByteTmp = (0x000000FF & ((int) data[4 + j]));
                        ByteTmp = (char) (FirstByteTmp << 8 | SecondByteTmp);

Is it possible to duplicate sections of code in OOP and is it possible to remove these duplicates somehow without extra crutches?

Answer the question

In order to leave comments, you need to log in

5 answer(s)
D
dude2012, 2016-03-11
@dude2012

It's not even a noob thing. But in the practical part. It's better to fix one code than two. But in fact, it's not as bad as forgetting to fix the second code if you fixed the first one. Therefore, the fewer corrections, the better - less headaches, fewer potential errors. A rake can hit hard on the head, not this time, next time. Better to have good habits.
I recommend reading "Refactoring" by Martin Fowler, there are many examples on this topic.
Just to remove duplicates is refactoring.

S
Sergey, 2016-03-11
Protko @Fesor

noobness. Move the common into methods, into common dependencies. Read about DRY.
Duplication is allowed if it is ... well, stupid code and there is no escape from duplication. But you have some expressions here, logic, behavior is duplicated. And this is noob.

E
Evhen, 2016-03-11
@EugeneP2

Regarding "switch (data.length)": here you need to use polymorphism. Create an interface with a method that accepts an int[]data array, and make an appropriate implementation for each case option.
for example

interface Worker {
  void doWork(int[] data);
}

class Worker96 implements Worker  {
   public void doWork(int[] data) {
       ....
   }
}

class Worker71 implements Worker  {
   public void doWork(int[] data) {
       ....
   }
}

// потом можно создать мапу 

class Main {

private Map<Integer, Worker> workers = ....;

Main (){
    workers.put(96, new Worker96());
    workers.put(71, new Worker71());
}
public void dataProcessing(int[] data) {
   Worker w = workers.get(data.length);
   if (w == null)
       throw new RuntimeException("Unsupported data length!");

   w.doWork(data);
}
}
// как то так...

Regarding the first case, if the code does not involve storing state,
then you can move the code into a separate utility class into a static method.

M
Michael, 2016-03-11
@Sing303

There is no duplication in pure code, you can always get rid of it, but it all depends on time.
There are situations when, in order to get rid of duplication, you need to completely redesign the project or sculpt crutches. Most often it happens like this:
- The boss comes, says to do the task, it needs to be done yesterday
- You look at the code and see that there is already a part of the solution, you need to use it elsewhere
, you rejoice - You try to use it, you understand, in order to pass this code, you need write a dozen classes and redo half of the project, you get frustrated, obviously you can’t do it in a day, the deadlines are running out This happens, most often, with legacy code
out - One of 2 options, or you cut a crutch so that the code is still in one place, or you stupidly copy it))
- Done

D
Dmitry Alexandrov, 2016-07-01
@jamakasi666

I strongly recommend reading "Design Patterns" (O'relly Eric and Elizabeth Freeman), a lot of these questions will disappear immediately and writing will be much easier.
Abstract classes are provided for your situation. For example:

public abstract class MyClass{
//Ваши общие методы реализованны тут
// Подсчет тока по каждому двигателю
    private float getCurrent(int engine){
        float phasesA = calculatePhaseForEngine(0, MassCurrentsPhases, engine);
        float phasesB = calculatePhaseForEngine(1, MassCurrentsPhases, engine);
        float phasesC = calculatePhaseForEngine(2, MassCurrentsPhases, engine);

        return (phasesA + phasesB + phasesC) / 3;
    }
    // Подсчетываем ток по выбраному двигателю
    private static float calculatePhaseForEngine(int i, float[] MassCurrentsPhases, int currentEng) {
        return MassCurrentsPhases[3 * currentEng + i];
    }
public abstract void doSomething(); //Такие методы наследники будут обязаны реализовать
}

Then you inherit from the abstract class and implement methods that differ in logic. The code becomes smaller, it is read and understood better and it is already more difficult to screw up. common methods are guaranteed to be the same for everyone.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question