A
A
Andrey Taranov2015-11-02 11:29:43
Programming
Andrey Taranov, 2015-11-02 11:29:43

What is wrong with my C code?

One American company is looking for a team leader - an embedded systems developer. I approached on the basis of experience and skills, but they refused to talk with me further after I sent a piece of C code for the microcontroller at their request. the contents of the main file are here: ideone.com/V7dhJy The code is working. I didn’t specially caramelize it, I sent it as it was immediately after debugging.
Tell me what is so terrible in my code?

Answer the question

In order to leave comments, you need to log in

3 answer(s)
O
OnYourLips, 2015-11-02
@OnYourLips

Start from comments: they denote bad code.
Variables with short names (comment turns into variable names)
Methods of 100+ lines (comments turn into names of short methods).
The code is very dirty. It is necessary to remove code that does nothing, and not to comment on it. This reason is already enough to refuse.
Examples:

if (flagBlink == 0)
  {
    //LED_BOARD_ON;
  }
  else
  {
    //LED_BOARD_OFF;
  }	
}

void TIMblink_IRQHandler(void)
{
  MAIN_Routine_Blink();
 
  //HAL_TIM_IRQHandler(&hTIMblink);
}

//for (ii=0;ii<100;ii++)
    {
    }

L
LeEnot, 2015-11-02
@LeEnot

I say right away that I don’t really understand C and controllers, and I don’t evaluate you as a specialist. All of the following is just my guess, nothing personal.
Perhaps for the garbage in the code and the lack of named constants:

puTX.bytes[0]=(!IN_1<<0)+
                        (!IN_2<<1)+
                        (!IN_3<<2)+
                        (!IN_4<<3)+
                        (!IN_5<<4)+
                        (!IN_6<<5)+
                        (!IN_7<<6)+
                        (!IN_8<<7);

Compare, for example, with a piece of code from STM from your own example:
InitGPIO(GPIOB,GPIO_PIN_7,GPIO_MODE_INPUT); //1
  InitGPIO(GPIOB,GPIO_PIN_6,GPIO_MODE_INPUT);
  InitGPIO(GPIOB,GPIO_PIN_5,GPIO_MODE_INPUT);
  InitGPIO(GPIOB,GPIO_PIN_4,GPIO_MODE_INPUT);

Well, or this:
if (reset >= 2)
          {
            reset = 0;
            timeout = MAX_TIMEOUT;
            //PCKG_SwitchOffAllLeds(); // выключить все лампы
          }
          else
          {
            reset++;
          }

this is a junior code, not a team lead, which should do the review code for the juniors...

A
asd111, 2015-11-02
@asd111

Comments in the code in Russian.
Imagine that they are in Chinese and everything will become clear.
The rest is irrelevant if the code is working.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question