문제

I have a function that translates data using std::map

struct HistoParameter
{
  int nbins;
  float first;
  float last;
  HistoParameter(int _nbins, int _first, int _last) :
    nbins(_nbins), first(_first), last(_last) {};
};

HistoParameter* variable_to_parameter(char* var_name)
{
  std::map<const std::string, HistoParameter*> hp;
  hp[std::string("ph_pt")] = new HistoParameter(100,0,22000);
  hp[std::string("ph_eta")] = new HistoParameter(100,-3,3);
  // ...
  return hp[var_name];
}

My struct is very light, but image it can be heavy. The prolem is that every time I call this function it create a lot of HistoParameter objects, maybe a switch case is more efficient. First question: I'm creating garbage?

Second solution:

bool first_time = true;
HistoParameter* variable_to_parameter(char* var_name)
{
  static std::map<const std::string, HistoParameter*> hp;
  if (first_time)
    {
  hp[std::string("ph_pt")] = new HistoParameter(100,0,22000);
  hp[std::string("ph_eta")] = new HistoParameter(100,-3,3);
  // ...
    }
  first_time = false;
  return hp[var_name];

is it ok? Better solution?

도움이 되었습니까?

해결책

Your second solution should certainly improve efficiency, but isn't (at least IMO) the best implementation possible. First of all, it makes first_time publicly visible, even though only variable_to_parameter actually cares about it. You've already made hp a static variable in the function, and first_time should be as well.

Second, I would not use pointers and/or dynamic allocation for the HistoParameter values. At one int and two floats, there's simply no reason to do so. If you're really passing them around so much that copying became a problem, you'd probably be better off using some sort of smart pointer class instead of a raw pointer -- the latter is more difficult to use and much more difficult to make exception safe.

Third, I'd consider whether it's worthwhile to make variable_to_parameter into a functor instead of a function. In this case, you'd initialize the map in the ctor, so you wouldn't have to check whether it was initialized every time operator() was invoked. You can also combine the two, by have a static map in the functor. The ctor initializes it if it doesn't exist, and operator() just does a lookup.

Finally, I'd note that map::operator[] is primarily useful for inserting items -- it creates an item with the specified key if it doesn't exist, but when you're looking for an item, you usually don't want to create an item. For this, you're generally better off using map.find() instead.

다른 팁

브라우저로 이동하여 화살표를 클릭하여 웹 파트를 수정하십시오.고급 섹션을 확장하고 제목 URL을 원하는 URL로 바꿉니다.

와 같은 일을해야합니다.

private static List<Items> data;

public static List<Items> getData() {
   if(data==null) {
        //get from db
   }
   return data;
}
.

이렇게하면 데이터를 채우기 위해 데이터베이스에 하나의 트립이 하나만 있습니다.대안 적으로 저지를 사용하고 있으므로 Application 등록 할 때 그런 식으로 데이터 채우기

   <init-param>
       <param-name>javax.ws.rs.Application</param-name>
       <param-value>com.foo.MyApplication</param-value>
   </init-param> 
.

I'd have a std::map< std::string, HistoParameter *> member and do

InitializeHistoParameter() 
{
   myMap["ph_pt"] = new ...
   myMap["ph_eta"] = new ...
}

And then

HistoParameter* variable_to_parameter(char* var_name) 
{
    return myMap[var_name];
}

either way, you are creating memory leak. each time the = operator is called, for example:

hp[std::string("ph_pt")] = new HistoParameter(100,0,22000);

you are creating a new HistoParameter object and pairing the key "ph" with this most recent object, leaving the previous one dangling. If creating a new object each time is your actual intent, you probably need to call

delete hp[std::string("ph_pt")]; 

before the new operation.

My suggestion is to avoid raw new operations as much as possible and resort to smart pointers such as boost::share_ptr for object life time management.

라이센스 : CC-BY-SA ~와 함께 속성
제휴하지 않습니다 StackOverflow
scroll top