CODEWITHSUNDEEP
c++
Gavyn
Gavyn

Reputation: 21

Elegant alternative to looping over the same data?

Got a bit of a dilemma. 

std::map<myClass, int>                  myMap;


void Distribute(){
    float pool = 50;
    int receivers = 0;
    for(auto i = myMap.begin(); i != myMap.end(); i++){
        if(i->second == 1) receivers++;
    }

    float distribution = pool/receivers;

    for(auto i = myMap.begin(); i!= myMap.end; i++){
        if(i->second == 0)continue;
        i->first.value = distribution;
    }
}

Basically, what I'm trying to do is find the total size of the map, minus the elements with mapped values of 0. After that, I want to loop over the same map again, but send values to each entry using the data I collected from the last for loop.

This feels really ugly and inefficient. Surely there has to be some way of handling it all in 1 for loop? Or maybe the first for loop ideally wouldn't be necessary? I'm not against extra work, but I can't help feeling I'm writing ugly code here, I'd really like some input.

Upvotes: 1

Views: 96

Answers (2)

Andrew Lazarus
Andrew Lazarus

Reputation: 19340

Assuming everything is single-threaded.

You have to do the loop twice, but you can store the addresses of the elements that need to be changed so you don't have to test twice.

Note there is a very ugly const_cast in this code. It is ugly for a reason. You don't want to change the key of a map; it can break the map's behavior. I suspect you can simplify and improve your design by taking the second element (the 0 or 1 indicator) and making it a field in myClass. Then you don't need a map, just a set, vector, or list of myClass.

I also had to define a hash and operator== for myClass. I've omitted that part to show the important section.

void Distribute(){
    float pool = 50;
    std::vector<myClass*> nonzeros;
    for (auto iter = myMap.begin(); iter != myMap.end(); ++iter) {
        if (iter->second == 1) { nonzeros.push_back(const_cast<myClass *>(&(iter->first))); }
    }
    if (nonzeros.empty()) return;
    const float distribution = pool/nonzeros.size();
    std::for_each(nonzeros.begin(), nonzeros.end(),
        [distribution](decltype(nonzeros)::value_type pClass) {pClass->value = distribution;});
}

Upvotes: 0

May
May

Reputation: 121

if your data is inserted by yourself, you can use a wrapper class contains your map

class Wrapper{
public:
  int receivers = 0;
  std::map<myClass, int> myMap;// be sure you have operator< for myClass
  void insert(pair<myClass, int> item){
    myMap.insert(item);
    // count receivers when you insert , get receivers in constant time
    if(item.second == 1){
      receivers++;
    }
  }
};

void Distribute(){
  float pool = 50;
  Wrapper wrapper;
//insert your items by wrapper.insert(..) , not wrapper.myMap.insert(..)
  float distribution = pool/wrapper.receivers;
  for(auto& [i, j]: wrapper.myMap){
    if(j == 1){
      i.value = distribution;// be sure value is a public member of myClass
    }
  }
}

Upvotes: 2

Related Questions