why this code doesn't work properly - close orders on certain pips

[shermilden5000]

I made this ea to close all open trades when total pips reach +30, but it doesn't seem to be working, what's wrong with the code?

//+------------------------------------------------------------------+
//|                                               close on pips1.mq4 |
//|                        Copyright 2017, MetaQuotes Software Corp. |
//|                                             https://www.mql5.com |
//+------------------------------------------------------------------+
#property copyright "Copyright 2017, MetaQuotes Software Corp."
#property link      "https://www.mql5.com"
#property version   "1.00"
#property strict
//+------------------------------------------------------------------+
//| Expert initialization function                                   |
//+------------------------------------------------------------------+
int OnInit()
  {
//--- create timer
   EventSetTimer(60);
      
//---
   return(INIT_SUCCEEDED);
  }
//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
  {
//--- destroy timer
   EventKillTimer();
      
  }
//+------------------------------------------------------------------+
//| Expert tick function                                             |
//+------------------------------------------------------------------+
void OnTick()
  {
//---
   
  }
//+------------------------------------------------------------------+
//| Timer function                                                   |
//+------------------------------------------------------------------+
void OnTimer()
  {
//---
   
  }
//+------------------------------------------------------------------+

int start()
{
  
  double TotalPips = 0;

for(int otsb=OrdersTotal()-1;otsb>=0;otsb--)                      //Check through open orders
      {     if(OrderSelect(otsb,SELECT_BY_POS,MODE_TRADES)==true)     
     {
       
         double  pipsare= (OrderOpenPrice()-Bid)/(Point*10);        //Calculate PipValue for each trade       
        
      TotalPips=TotalPips+pipsare;
                
        }
        }
        
        
        if (TotalPips>30) {
        
        //*******
        int total = OrdersTotal();
  for(int i=total-1;i>=0;i--)
  {
    OrderSelect(i, SELECT_BY_POS);
    int type   = OrderType();

    bool result = false;
    
    switch(type)
    {
      //Close opened long positions
      case OP_BUY       : result = OrderClose( OrderTicket(), OrderLots(), MarketInfo(OrderSymbol(), MODE_BID), 5, White );
                          break;
      
      //Close opened short positions
      case OP_SELL      : result = OrderClose( OrderTicket(), OrderLots(), MarketInfo(OrderSymbol(), MODE_ASK), 5, White );
                           
    }
    
    if(result == false)
    {
      Alert("Order " , OrderTicket() , " failed to close. Error:" , GetLastError() );
      Sleep(3000);
    }  
     
    
  }
  
  return(0);
        
       } 
                
        
  
  return(0);
 
  
}

[William Roeder]

1. Use the new Event Handling Functions - Functions - Language Basics - MQL4 Reference or use the old style. Do not use both.

2. double  pipsare= (OrderOpenPrice()-Bid)/(Point*10);

   Why Bid? What if it a sell order? Use OrderClosePrice().

3. Code breaks on 4 digit brokers. Compute what a pip is. You are not adjusting SL, TP, and slippage; for 4/5 digit brokers and for JPY pairs.	How to manage JPY pairs with parameters? - MQL4 and MetaTrader 4 - MQL4 programming forum

   ---

4.  case OP_SELL      : result = OrderClose( OrderTicket(), OrderLots(), MarketInfo(OrderSymbol(), MODE_ASK), 5, White );
   Use OrderClosePrice() and you don't have to check it's type.

   ---

5. for(int i=total-1;i>=0;i--){
       OrderSelect(i, SELECT_BY_POS);

   In the presence of multiple orders (one EA multiple charts, multiple EAs, manual trading,) because while you are waiting for the current operation (closing, deleting, modifying) to complete, any number of other operations on other orders could have concurrently happened and changed the position indexing:

   1. For non-FIFO (US brokers,) (or the EA only opens one order per symbol,) you can simply count down in a position loop, and you won't miss orders. Get in the habit of always counting down.	Loops and Closing or Deleting Orders - MQL4 forum

   2. For FIFO (US brokers,) and you (potentially) process multiple orders, you must count up and on a successful operation, reprocess all positions (set index to -1 before continuing.)

   3. and check OrderSelect.	What are Function return values ? How do I use them ? - MQL4 forum
      	Common Errors in MQL4 Programs and How to Avoid Them - MQL4 Articles

   4. and if you (potentially) process multiple orders, must call RefreshRates() after server calls if you want to use the Predefined Variables (Bid/Ask) or OrderClosePrice() instead, on the next order/server call.