Оптимизация кода

vvadim
Offline
Зарегистрирован: 23.05.2012

В программировании слабоват.
Есть вот такой кусок рабочего кода.

 if(count_LAST == 2)    Pos_LAST = Pos2;
  if(count_LAST == 3)    Pos_LAST = Pos3;
  if(count_LAST == 4)    Pos_LAST = Pos4;
  if(count_LAST == 5)    Pos_LAST = Pos5;
  if(count_LAST == 6)    Pos_LAST = Pos6;
  if(count_LAST == 7)    Pos_LAST = Pos7;
  if(count_LAST == 8)    Pos_LAST = Pos8;
  if(count_LAST == 9)    Pos_LAST = Pos9;
  if(count_LAST == 10)   Pos_LAST = Pos10;
  if(count_LAST == 11)   Pos_LAST = Pos11;  
  if(count_LAST == 12)   Pos_LAST = Pos12;
  if(count_LAST == 13)   Pos_LAST = Pos13;
  if(count_LAST == 14)   Pos_LAST = Pos14;
  if(count_LAST == 15)   Pos_LAST = Pos15;
  if(count_LAST == 16)   Pos_LAST = Pos16;
  if(count_LAST == 17)   Pos_LAST = Pos17;
  if(count_LAST == 18)   Pos_LAST = Pos18;
  if(count_LAST == 19)   Pos_LAST = Pos19;
  if(count_LAST == 20)   Pos_LAST = Pos20;
  if(count_LAST == 21)   Pos_LAST = Pos21;
  if(count_LAST == 22)   Pos_LAST = Pos22;
  if(count_LAST == 23)   Pos_LAST = Pos23;
  if(count_LAST == 24)   Pos_LAST = Pos24;
  if(count_LAST == 25)   Pos_LAST = Pos25;
  if(count_LAST == 26)   Pos_LAST = Pos26;
  if(count_LAST == 27)   Pos_LAST = Pos27;
  if(count_LAST == 28)   Pos_LAST = Pos28;
  if(count_LAST == 29)   Pos_LAST = Pos29;
  if(count_LAST == 30)   Pos_LAST = Pos30;
  if(count_LAST == 31)   Pos_LAST = Pos31;
  if(count_LAST == 32)   Pos_LAST = Pos32;
  if(count_LAST == 33)   Pos_LAST = Pos33;
  if(count_LAST == 34)   Pos_LAST = Pos34;
  if(count_LAST == 35)   Pos_LAST = Pos35;
  if(count_LAST == 36)   Pos_LAST = Pos36;
  if(count_LAST == 37)   Pos_LAST = Pos37;
  if(count_LAST == 38)   Pos_LAST = Pos38;
  if(count_LAST == 39)   Pos_LAST = Pos39;
  if(count_LAST == 40)   Pos_LAST = Pos40;
  if(count_LAST == 41)   Pos_LAST = Pos41;
  if(count_LAST == 42)   Pos_LAST = Pos42;
  if(count_LAST == 43)   Pos_LAST = Pos43;
  if(count_LAST == 44)   Pos_LAST = Pos44;
  if(count_LAST == 45)   Pos_LAST = Pos45;
  if(count_LAST == 46)   Pos_LAST = Pos46;
  if(count_LAST == 47)   Pos_LAST = Pos47;
  if(count_LAST == 48)   Pos_LAST = Pos48;
  if(count_LAST == 49)   Pos_LAST = Pos49;
  if(count_LAST == 50)   Pos_LAST = Pos50;

Возможно ли сделать запись более короткой?

Datak
Offline
Зарегистрирован: 09.10.2014

Можно конечно. Не совсем понятно, что такое эти Pos2, Pos3, и т.д - но в общем всё может выглядеть как-то так:

















int Pos[ ] = { 0, 0, Pos2, Pos3, Pos4, ....... , Pos50 }; // или, по возможности, const int

.....
.....

Pos_LAST = Pos[ count_LAST ];

 

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

Зависит от значений Pos*, если есть зависимость, то можно упростить (формализовать) в каком то виде, если нет, то через массив, как уже сказали.

Немного не в кассу. Когда пишите несколько if, лучше писать if... else if... else if... else ; в противном случае у Вас программа будет проходить через все ифы, а оно Вам надо? (риторический). Другой вариант switch. Увы, не помню, что оптимальней, вроде какие то были отличия (switch от if else if else if ; ) или грабли, может кто подскажет.

 

vvadim
Offline
Зарегистрирован: 23.05.2012

А зачем в начале массива два нуля?

Datak
Offline
Зарегистрирован: 09.10.2014

vvadim пишет:

А зачем в начале массива два нуля?

Ну у вас же  вкоде сравнение начинается с двойки

if(count_LAST == 2)    Pos_LAST = Pos2;

поэтому для первых двух значений - 0 и 1 - оставили "пустые" ячейки.

Как вариант - можно было и без них обойтись. Тогда было бы так:

Pos_LAST = Pos[ count_LAST - 2 ];

 

Datak
Offline
Зарегистрирован: 09.10.2014

kisoft пишет:

Другой вариант switch. Увы, не помню, что оптимальней, вроде какие то были отличия (switch от if else if else if ; ) или грабли, может кто подскажет.

Да вряд ли тут кто подскажет. Зависит от конкретных значений в switch, и от конкретного компилятора. И даже от опций заданных при компиляции.

Иногда switch может компилиться в такую же таблицу переходов,  а иногда в те же if - else. Не угадаешь. :)

vvadim
Offline
Зарегистрирован: 23.05.2012

С массивом попробую, а вот такие условия 


  if(Rotor_GO > Pos1 && Rotor_GO <= Pos2)      servoGO = map(Rotor_GO, Pos1, Pos2, servo1, servo2);
  if(Rotor_GO > Pos2 && Rotor_GO <= Pos3)      servoGO = map(Rotor_GO, Pos2, Pos3, servo2, servo3);
  if(Rotor_GO > Pos3 && Rotor_GO <= Pos4)      servoGO = map(Rotor_GO, Pos3, Pos4, servo3, servo4);
  if(Rotor_GO > Pos4 && Rotor_GO <= Pos5)      servoGO = map(Rotor_GO, Pos4, Pos5, servo4, servo5);
  if(Rotor_GO > Pos5 && Rotor_GO <= Pos6)      servoGO = map(Rotor_GO, Pos5, Pos6, servo5, servo6);
  if(Rotor_GO > Pos6 && Rotor_GO <= Pos7)      servoGO = map(Rotor_GO, Pos6, Pos7, servo6, servo7);
  if(Rotor_GO > Pos7 && Rotor_GO <= Pos8)      servoGO = map(Rotor_GO, Pos7, Pos8, servo7, servo8);
  if(Rotor_GO > Pos8 && Rotor_GO <= Pos9)      servoGO = map(Rotor_GO, Pos8, Pos9, servo8, servo9);
  if(Rotor_GO > Pos9 && Rotor_GO <= Pos10)     servoGO = map(Rotor_GO, Pos9, Pos10, servo9, servo10);

 

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

Навскидку, массив структур, бинарный поиск, функция пересчета. Хотя можно и простым поиском обойтись, значений немного. Если других вариантов не будет, покажу пример.

Datak
Offline
Зарегистрирован: 09.10.2014
int Pos[ ] = { Pos1, Pos2, Pos3, ....... , Pos10 };          // или, по возможности, const int
int Servo[ ] = { Servo1, Servo2, Servo3, ....... , Servo9 }; // 

.....
.....

for( int i = 0; i < ( sizeof( Servo ) / sizeof( Servo[ 0 ] ) ); i++ )
{
  int Pos_L = Pos[ i ];
  int Pos_H = Pos[ i + 1 ];

  if( Rotor_G0 > Pos_L && Rotor_G0 <= Pos_H )
  {
    ServoG0 = map( Rotor_G0, Pos_L, Pos_H, Servo[ i ], Servo[ i + 1 ] );
    break;
  }
}

Это если так, "в лоб", по-простому. В зависимости от реального размера массивов и от требований к скорости - да, можно попробовать что-то как-то оптимизировать.

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

Удобней объединять в структуру, а не использовать два разных массива, будет меньше ошибок, уж поверьте

 

vvadim
Offline
Зарегистрирован: 23.05.2012

Со структурами вообще тёмный лес

Попробую поиграться с массивами

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

/*
* Исходные значения неизвестны, ставим как попало, но в возрастающем порядке
*/

#define Pos1 30
#define Pos2 38
#define Pos3 49
#define Pos4 70
#define Pos5 75

#define servo1 500
#define servo2 700
#define servo3 900
#define servo4 1000
#define servo5 1300

#define NO_POS 0
#define NO_VALUE 0

typedef struct _ServoVals
{
	int		pos_start;		/* Верхняя граница */
	int		servo_min;		/* Минимум */
	int		servo_max;		/* Максимум */
} ServoVals;

const int pos_minimum = Pos1;

const ServoVals array[] =
{
	{ Pos2, servo1, servo2 },
	{ Pos3, servo2, servo3 },
	{ Pos4, servo3, servo4 },
	{ Pos5, servo4, servo5 }
};

/* Длина массива, т.е. кол-во записей */
const int array_size = sizeof(array) / sizeof(ServoVals);

/* Преобразование, возвращает NO_VALUE, если значение выходит за пределы */
int getServValue(int Rotor_GO)
{
	/* Сначала проверяем нижнюю границу, всё, что меньше - неинтересно */
	int l_PrevPos = pos_minimum;
	if (Rotor_GO <= l_PrevPos)
	{
		return NO_VALUE;	// Здесь непонятно что возвращать, потому что исходный код не полный
	}
	/* Дальше проверка по всем структурам, расположенным в массиве, начиная с первого элемента */
	for (int i = 0; i < array_size; ++i)
	{
		if (Rotor_GO <= array[i].pos_start)
		{
			return map(Rotor_GO, l_PrevPos, array[i].pos_start, array[i].servo_min, array[i].servo_max);
		}
		l_PrevPos = array->pos_start;
	}
	return NO_VALUE;
}

void setup()
{
	Serial.begin(57600);
}

int val = 40;

void loop()
{
	Serial.print("Rotor_GO = ");
	Serial.print(val);

	/* Вызываем преобразование */
	int servoGO = getServValue(val);
	/* Проверка на наличие реального ответа */
	if (servoGO != NO_VALUE)
	{
		Serial.print(", ServoGO = ");
		Serial.println(servoGO);
	}
	else
	{
		Serial.println(", NO_VALUE");
	}
	delay(1000);
	
	/* Меняем исходное значение для проверки */
	val += rand();
	val %= 100;
}

Вот такой вариант со структурой. Не смотри, что много, больше кода для тестирования.

В скетче #8 есть как минимум одна ошибка, выход за пределы массива. Аккуратней.

 

vvadim
Offline
Зарегистрирован: 23.05.2012

kisoft пишет:


В скетче #8 есть как минимум одна ошибка, выход за пределы массива. Аккуратней.

 

Имеете ввиду Servo[ i + 1 ] при достижении максимального значения i

vvadim
Offline
Зарегистрирован: 23.05.2012

В пример со структурами попробую вникнуть...

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

vvadim пишет:

kisoft пишет:


В скетче #8 есть как минимум одна ошибка, выход за пределы массива. Аккуратней.

 

Имеете ввиду Servo[ i + 1 ] при достижении максимального значения i


Ну да, везде, где есть индекс+1

Datak
Offline
Зарегистрирован: 09.10.2014

kisoft пишет:

В скетче #8 есть как минимум одна ошибка, выход за пределы массива. Аккуратней.

У меня что ли? А, ну да, есть.

Монитор потому что маленький - десятая серва перенеслась на другую строку, и я её не заметил. :)

Конечно, массивы должны быть одинакового размера, а цикл на единицу короче.

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

vvadim пишет:

В пример со структурами попробую вникнуть...


Пример рабочий, проверял. Отвечу на вопросы, если понадобится. А нет, так нет :)

vvadim
Offline
Зарегистрирован: 23.05.2012

Спасибо откликнувшимся.
Решил сначала разобраться с массивами, на будуще однозначно пригодится , да и простыни исписывать не хочется (код уже переделал и костыль с размером массива выскочил, но вроде обошёл его).
Потом полезу в структуры.

 

 

vvadim
Offline
Зарегистрирован: 23.05.2012

 

if(Rotor_GO > Pos1 && Rotor_GO <= Pos2)      servoGO = map(Rotor_GO, Pos1, Pos2, servo1, servo2);
if(Rotor_GO > Pos2 && Rotor_GO <= Pos3)      servoGO = map(Rotor_GO, Pos2, Pos3, servo2, servo3);
if(Rotor_GO > Pos3 && Rotor_GO <= Pos4)      servoGO = map(Rotor_GO, Pos3, Pos4, servo3, servo4);
if(Rotor_GO > Pos4 && Rotor_GO <= Pos5)      servoGO = map(Rotor_GO, Pos4, Pos5, servo4, servo5);
if(Rotor_GO > Pos5 && Rotor_GO <= Pos6)      servoGO = map(Rotor_GO, Pos5, Pos6, servo5, servo6);
if(Rotor_GO > Pos6 && Rotor_GO <= Pos7)      servoGO = map(Rotor_GO, Pos6, Pos7, servo6, servo7);
if(Rotor_GO > Pos7 && Rotor_GO <= Pos8)      servoGO = map(Rotor_GO, Pos7, Pos8, servo7, servo8);
if(Rotor_GO > Pos8 && Rotor_GO <= Pos9)      servoGO = map(Rotor_GO, Pos8, Pos9, servo8, servo9);
if(Rotor_GO > Pos9 && Rotor_GO <= Pos10)     servoGO = map(Rotor_GO, Pos9, Pos10, servo9, servo10);

Эту простыню записал так

 

for(int i=0; i <= 9; i++ )
  {
    if( Rotor_GO > Pos[i] && Rotor_GO <= Pos[i + 1] )
    {
      servoGO = map(Rotor_GO, Pos[i], Pos[i + 1], servo[i], servo[i + 1]); 
      break;
    }
}

Вроде получилось.
Но есть и такая запись.

 if ( BUTTON_SAVEState == HIGH && count == 0) 
{
  count = 1;
  Pos[0] = number;
  Pos_LAST = Pos[0];
  servo[0] = valPot;
  BUTTON_SAVEState = LOW;
}
if ( BUTTON_SAVEState == HIGH && count == 1) 
{
  count = 2;
  Pos[1] = number;
  Pos_LAST = Pos[1];
  servo[1] = valPot;
  BUTTON_SAVEState = LOW;
}

if ( BUTTON_SAVEState == HIGH && count == 2) 
{
  count = 3;
  Pos[2] = number;
  Pos_LAST = Pos[2];
  servo[2] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 3) 
{
  count = 4;
  Pos[3] = number;
  Pos_LAST = Pos[3];
  servo[3] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 4) 
{
  count = 5;
  Pos[4] = number;
  Pos_LAST = Pos[4];
  servo[4] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 5) 
{
  count = 6;
  Pos[5] = number;
  Pos_LAST = Pos[5];
  servo[5] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 6) 
{
  count = 7;
  Pos[6] = number;
  Pos_LAST = Pos[6];
  servo[6] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 7) 
{
  count = 8;
  Pos[7] = number;
  Pos_LAST = Pos[7];
  servo[7] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 8) 
{
  count = 9;
  Pos[8] = number;
  Pos_LAST = Pos[8];
  servo[8] = valPot;
  BUTTON_SAVEState = LOW;
} 
if ( BUTTON_SAVEState == HIGH && count == 9) 
{
  count = 10;
  Pos[9] = number;
  Pos_LAST = Pos[9];
  servo[9] = valPot;
  BUTTON_SAVEState = LOW;
}
if ( BUTTON_SAVEState == HIGH && count == 10) 
{
  count = 11;
  Pos[10] = number;
  Pos_LAST = Pos[10];
  servo[10] = valPot;
  BUTTON_SAVEState = LOW;
}

Условия работают при разных режимах. Вот их увязать и не получается.
Конечно не катастрофа, но хотелось бы узнать как в данном случае правильно записать.

 

vvadim
Offline
Зарегистрирован: 23.05.2012

kisoft пишет:
 Пример рабочий, проверял. Отвечу на вопросы, если понадобится. А нет, так нет :)

Пока сложновато, но мне интересно, буду ковыряться...

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

Сделать одной функцией: sorry, я с мобилы, форматировать сложно, главное - смысл:
addNewPoint()
{
if ( BUTTON_SAVEState == HIGH )
{
Pos[count] = number;
Pos_LAST = Pos[count];
servo[count] = valPot;
BUTTON_SAVEState = LOW;
count++;
}
}

vvadim
Offline
Зарегистрирован: 23.05.2012

Спасибо за идею.
А вот как эту функцию теперь запускать?
Получается условие в самой функции

 

kisoft
kisoft аватар
Offline
Зарегистрирован: 13.11.2012

Это уж тебе видней, потому что любая задача решается комплексно, а ты даёшь нам кусочки, потому сложно сделать оптимально задачу вцелом.
А вызывать нужно где то в loop, плюс добавить проверку на длину массива, а то count не ограничен. Плюс здесь используются глобальные переменные, что не очень хорошо, плюс это можно не делать функцией, а просто вписать этот код в нужном месте.
На счёт условия внутри функции, вариант от телепата, возвращать условие из функции