dim2::Point default constructor

[Peter Kovacs]

I think the default constructor of dim2::Point should set the point (0,0). It is a natural request for any class that provides operator+ to be able to sum it correctly with a template function.

I also made benchmark tests. They showed that this variant is not slower than the current one.

[Peter Kovacs]

[03b3e59f0b4f] contains this modification.

[Alpar Juttner]

Replying to kpeter:

I think the default constructor of dim2::Point should set the point (0,0). It is a natural request for any class that provides operator+ to be able to sum it correctly with a template function.

I think it is far from being a natural request.

I also made benchmark tests. They showed that this variant is not slower than the current one.

Which kind of benchmark did you made?

Alpar

[Alpar Juttner]

Probably it is better to raise a topic like this on the mailing list first.

[Peter Kovacs]

Replying to alpar:

I think it is far from being a natural request.

We have an argument about this question months ago. Like it or not it is an important request in C++. Please try to write a template function that can sum int, double etc. values as well as dim2::Point<T> values correctly without this modification.

If we can't write such a function, then it is proved that the suggested default constructor is a natural request.

Replying to alpar:

Which kind of benchmark did you made?

E.g. I created very large std::vector<dim2::Point<T> > structures using int, double etc. as T.

[Peter Kovacs]

Replying to alpar:

Probably it is better to raise a topic like this on the mailing list first.

Why?

Small topics like that are usually neglected and then forgotten on the list. I think this ticket system is for tickets like that (among others). A ticket can be closed with wontfix status, if it is not reasonable.

[Alpar Juttner]

Replying to kpeter:

Replying to alpar:

I think it is far from being a natural request.

We have an argument about this question months ago. Like it or not it is an important request in C++. Please try to write a template function that can sum int, double etc. values as well as dim2::Point<T> values correctly without this modification.

Well, let me try. What about this?

template<class T, class I>
T sum(I begin, I end, const T &init)
{
  T s(init);
  for(I i=begin;i!=end;++i) s+=*i;
  return s;
}

This is probably the only correct approach.

If we can't write such a function, then it is proved that the suggested default constructor is a natural request.

As you can see above, we can. What then?

[Alpar Juttner]

So, I'm not convinced at all neither to have a default constructor nor to keep the present setup.

However, it isn't break the API if we change it later, thus we can safely postpone this ticket after the 1.0 release.

[Peter Kovacs]

Replying to alpar:

Well, let me try. What about this?

You are right. I should have specified the problem more exactly: I'm looking for a template sum function without getting an explicit null-value for T, since I think your approach is uncomfortable in many cases.

See e.g. the maps_summary.cc demo file in the svn repo. As far as I know the suggested standard way of summing in C++ is the one that is used in the summary() function. It not works for dim2::Point.

In fact, it is not clear enough whether this change is necessary or not, but it is clear that it have advantages, and it doesn't have any disadvantage. Am I right? If it is true, than I think it should be accepted.

[Alpar Juttner]

Replying to kpeter:

Replying to alpar:

Well, let me try. What about this?

You are right. I should have specified the problem more exactly: I'm looking for a template sum function without getting an explicit null-value for T, since I think your approach is uncomfortable in many cases.

I understand, but typically you do not want to do it in this form if you want to have a uniform interface of the similar functions (i.e. creating an aggregate value of a container using an aggregating operator.) Think of e.g. max and min.

See e.g. the maps_summary.cc demo file in the svn repo. As far as I know the suggested standard way of summing in C++ is the one that is used in the summary() function.

Hopefully not.

In fact, it is not clear enough whether this change is necessary or not, but it is clear that it have advantages, and it doesn't have any disadvantage. Am I right?

If the second statement is true, then yes, you are right.

If it is true, than I think it should be accepted.

Nobody said the opposite (not me, at least). I just say that we have much more important things to do now. That is why it was postponed until the next release.

[Alpar Juttner]

Replying to alpar:

Nobody said the opposite (not me, at least). I just say that we have much more important things to do now. That is why it was postponed until the next release.

It seems to be still true.

[Peter Kovacs]

Could you tell me any disadvantage of this modification?

[Alpar Juttner]

Replying to kpeter:

Could you tell me any disadvantage of this modification?

My concerns are the followings.

• Conceptually bad. The purpose of a constructor should be to initialize the data structure and not to set it to some comfortable default value, especially for tiny structures like this. Though, I admit this misuse is quite frequent in C++.
• Potentially hides bugs. If a datastructure is used uninitialized, then the chance is high that the compiler will warn about it. But this patch​ hides this kind of bugs.
• Restricts the usage, as it requires the existence of an int->X conversion, where X is the underlying data type.
• May degrade performance due to double initialization.

Neither of these problem are serious. But the only thing I can see in the other pan of the balance is that it makes it possible to implement a sum() function with an improper interface. Quite weak.

[Peter Kovacs]

I think, only "Restricts the usage" could be a somewhat relevant reason of yours. But it is "quite weak".

I think, this discussion is not about performance or usability any more. It is rather about your personal antipathy against a common concept/guideline in C++. What you declared in the first point ("Conceptually bad") is not objective, it is a matter of taste.

In fact, I don't find this conceptual question so important, so I suggest closing this ticket with wontfix status.