How to get max of average??

[mnm71]

In this program class Sensor just get two value as sn & temp,each Sensor is a Network and two Network join together is a Region,in class Network I want to Calculation average of temp and in class Region I want to get max of average but i get just max of temp, what am i missing??
Network.cpp:

float Network::average_temp() {
	float sum = 0;
	for (int i = 0; i < (int) sensors.size(); i++) {
		sum = sum + sensors.at(i).getTemp();
	}
	float a = sum / (int) sensors.size();
	return a;

}

Region.cpp:

float Region::max_avg_temp() {
	float max = r_networks.at(0).average_temp();
	for (int i = 1; i < (int) r_networks.size(); i++) {
		if (r_networks.at(i).average_temp() > max)
			max = r_networks.at(i).average_temp();
	}
	return max;
}

[zpiecuch]

There are lot of places in this code that are potentially source of problems (like casting or empty collections etc.) but I don't think they are the goal of this question.

At first glance this code should return average.
Please provide example test set that you are using to test Region::max_avg_temp().
When you do unit tests of Network::average_temp() method do you get the average value or max one?

[mnm71]

I get the max value
and my main.cpp:

int main() {
	int t;
	Region region;
	for (cin >> t; t != 0; t--) {     // reads t different test cases .


		while (true) {
			int sn, temp;
			cin >> sn >> temp;
			if (sn == 0 && temp == 0)
				break;
			region.add_sensor(sn, temp);

		}

	}

		cout << region.num_of_networks() << " " << region.max_avg_temp()
				<< endl;

	return 0;
}

in line 26 i using Region::max_avg_temp().

[zpiecuch]

You didn't provide test set that you use for testing :(. I don't know what you enter when main is running.
If Network::average_temp() returns you max value then there is the problem.
Do you have more than one sensor in a network?
Modify average_temp() method in following way to check it

float Network::average_temp() {
	float sum = 0;
	cout << "Number of sensors in a network: " << (int)sensors.size() << "\n";
	for (int i = 0; i < (int) sensors.size(); i++) {
		sum = sum + sensors.at(i).getTemp();
	}
	float a = sum / (int) sensors.size();
	return a;

}

I would assume that you have only one sensor per network and suspect region.join_networks is working NOT as expected. Unit tests are your friends here.

[mnm71]

I use your way and my enter value:

2   //t
1 23  //input
2 14 //input
0 0
1 36 //input
2 6   //input
0 0

Number of sensors in a network: 1
Number of sensors in a network: 1
Number of sensors in a network: 1
Number of sensors in a network: 1
Number of sensors in a network: 1
4 36   //output 

My program have more function but I just have problem in average and max average temp function ,so my main has more modify ,I edit it above to cout just max average .

[zpiecuch]

You've modified your main() after I commented it - not nice :(
Taking what's here now it looks that the code is correct:
Every network has only one sensor so the max value is at the same time average value, therefore max average is at the same time max.
If you want average to be different than max then you need to have more than one sensor in networks.
In which place you add more thank one sensor to a network.
In previous version of your main there was region.join_networks method. Test if it really do what you intend.

Right now in previous comment everything looks as expected basing on the part of code you provided.

[keskiverto]

The "max of average" sounds confusing. How about:

I have N sets of values. Each set is represented by value A. I thus have N values Ai. I need the largest Ai.

Entirely separately:
The value A that represents a set is the average of the values in the set.


The way a problem is expressed does affect how one thinks about the problem and thus what kind of solutions one invents (first).


Why are the return values of *.size() cast to int? Why are the casts C-style casts and not C++-style casts?

[mnm71]

@zpiecuch I'm sorry I do it just for clear question and woooowwww you right I should use join function in average temp ,why Why I didn't care about I see just other hand of question :o :( ,in Network class join function ‫‪:add all of sensors in other network to this network and in Region class join function: two Network join together,are you have any idea for use join function in average temp function??

void Network::join(Network other_network) {
	j = 0;
	sensors.push_back(other_network.sensors[j]);
	sensors.insert(sensors.end(), other_network.sensors.begin(),
			other_network.sensors.end());

}

void Region::join_networks(int sn1, int sn2) {
	indexNetwork1=0;
	int allNet = (int) r_networks.size() + indexNetwork1;
	for (int i = 0; i < allNet; i++) {
		if (r_networks.at(i).contains(sn1)) {
			indexNetwork1 = i;
		} else if (r_networks.at(i).contains(sn2)) {
			indexNetwork2 = i;
		}
	}
	r_networks[indexNetwork1].join(r_networks[indexNetwork2]);
	indexNetwork1++;
	for (int i = indexNetwork2; i < allNet - 1; i++) {
		r_networks.at(i) = r_networks.at(i + 1);
	}
	//indexNetwork2--;
	allNet--;
}

int main() {
	int t;
	Region region;
	for (cin >> t; t != 0; t--) {     // reads t different test cases .


		while (true) {
			int sn, temp;
			cin >> sn >> temp;
			if (sn == 0 && temp == 0)
				break;
			region.add_sensor(sn, temp);

		}
		while (true) {
			int sn1, sn2;
			cin >> sn1 >> sn2;
			if (sn1 == 0 && sn2 == 0)
				break;
		if (!(region.same_network(sn1, sn2)))
		region.join_networks(sn1, sn2);

		}
	}

		cout << region.num_of_networks() << " " << region.max_avg_temp()
				<< endl;

	return 0;
}

[zpiecuch]

I don't even wanna know how does that code compile :/ There is a lot of potential problems. But lets get to the main question.

About network join, it's enough to have there only

sensors.insert(sensors.end(), other_network.sensors.begin(),
			other_network.sensors.end());

line. The other two are not necessary.

join_networks method could look like below. I'm however doing lot of assumptions about type of the variables and I did not try to compile it so something may still be wrong.

void Region::join_networks(int sn1, int sn2) {
	if ( sn1 == sn2 ) return;
	if ( r_networks.size() < 2 ) return;
	for(auto &net1 : r_networks) {
		if (net1.contains(sn1)) {
			for(auto &net2 : r_networks) {
				if (net2.contains(sn2)) {
					net1.join(net2);
					r_networks.erase(std::remove(r_networks.begin(), r_networks.end(), net2), r_networks.end());
					return;
				}
			}
		}
	}
	cout << "join_networks did not join anything";
}

You should have enough information to fix your code already.
Good luck.

[mnm71]

thank you I just change my average temp and my code work correctly....

float Network::average_temp() {
	float a=0;
	float sum = 0;
	for (i = 0; i <(int)sensors.size(); i++) {
		//sensors.push_back(sensors[i]);
		sum = sum+sensors[i].getTemp();
		a=sum/2;
		
	}
	 return a;
}

[keskiverto]

... and pigs fly with sufficient propulsion. It does, however, require extraordinary circumstances for the propulsion to be available.

Why is the line 7 inside the loop? Each iteration does overwrite the value of a, so other than last iteration do useless work. Besides, calculating something from incomplete sum has no purpose here; only the final result counts.


The real problem is on line 7 too. Are you sure that the average of sensors.size() values is always computed by dividing their sum by two?

[mnm71]

@keskiverto what's mean of this line (and pigs fly with sufficient propulsion. It does, however, require extraordinary circumstances for the propulsion to be available.)???
are you kidding me??

[keskiverto]

Your function produces correct result if and only if the sensors.size() is exactly two on every call of the function.

It seems quite possible that the precondition is not always true, just like it is quite possible that pigs cannot fly.

float Network::average_temp() {
  if ( 0 < sensors.size() ) {
    float sum = 0.0;
    for ( const auto & sensor : sensors.size() ) {
      sum += sensor.getTemp();
    }
    return ( sum / sensors.size() ); // once after loop, with real count
  }
  else {
    return 0.0f; // is 0 really proper average for nothing?
  }
}