Skip to content
This repository has been archived by the owner on Nov 29, 2018. It is now read-only.

Operator TimeFrame#& should not return nil #1

Closed
mblumtritt opened this issue May 24, 2014 · 13 comments
Closed

Operator TimeFrame#& should not return nil #1

mblumtritt opened this issue May 24, 2014 · 13 comments
Assignees

Comments

@mblumtritt
Copy link
Member

The result of an operator has to be bijective. If there is no way to return the expected type (TimeFrame) you should rather throw an exception.

@pderichs
Copy link
Contributor

Can you give an example where the operator does not behave as expected?
Where is the type issue? Do you mean the nil result in case of no intersection?

@mblumtritt
Copy link
Member Author

Yes, I mean the return of nil. That's unexpected and may result in follow-up errors.

@pderichs
Copy link
Contributor

We discussed the return value in case of non intersecting TimeFrame instances too, and we decided to go with the nil solution.

What would you expect to be returned if there is no intersection? An empty TimeFrame instance? Which time value for min should it contain?

It would be possible to define a time value like 01.01.1970 to be the indicator for these situations but IMO that would not be a nice solution.

Alternative solution: We could also create an instance of TimeFrame within the gem, which is empty by default and which would be returned in every case where an empty range should be returned - let's discuss about that.

In terms of code:

Current solution:

time_frame = TimeFrame.new(min: Time.new(2014), duration: 1.day)
other_time_frame = TimeFrame.new(min: Time.now, duration: 1.hour)
intersection = time_frame & other_time_frame
puts 'Intersecting!' if intersection

The empty TimeFrame solution:

time_frame = TimeFrame.new(min: Time.new(2014), duration: 1.day)
other_time_frame = TimeFrame.new(min: Time.now, duration: 1.hour)
intersection = time_frame & other_time_frame
puts 'Intersecting!' unless intersection.empty?

I like the first code sample a bit more, but this is a matter of personal taste - I am happy with both solutions in the end 😉

@bstoecker
Copy link

First of all: Coming from a mathematical background I have to say that this operator can never be bijective since the preimage is a space of dimension 2 and the image a space of dimension 1. So it is easy to generate four completely different timeframes t1 to t4 where t1 & t2 = t3 & t4. That means this operator is not injective and so also not bijective.

Lets get back to the topic. While having to timeframes t1 and t2 as imput we can have two scenarios: they cover a common timeframe or they don't. in case they cover a common timeframe, I think you agree with the result. So lets talk about the scenario they don't have a common timeframe. If they do not have a common timeframe, the result is (looking on it from a mathematical perspective with intervals) the emptyset (or nullset). So we decided to return nil which is the mathematical representative for empty set.

But lets discuss this approach. You suggest to throw an error, but from our current point from view this makes it a bit more difficult to handle it in code since you have to try catch (begin/rescue) it and (and this is more important to me) it's mathematically wrong, since the result is just the empty set. I think the only alternative which makes sence is a timeframe with no data. Such a timeframe should not have a min or a max value (timeframe.min = timeframe.max = nil). But such a change might also have some other strange impacts. What is the duration of such a timeframe? nil? 0? What is its behavior regarding the other methods (#without, #deviation_of...).

@bstoecker
Copy link

I also change the topic header, since this title is not correct

@bstoecker bstoecker changed the title result of operator TimeFrame#& is not bijective Operator TimeFrame#& should not return nil May 26, 2014
@jzernisch
Copy link
Contributor

I agree with @mblumtritt, as other Ruby core/stdlib classes like Array and Set have null objects, we should be consistent with that. An empty TimeFrame should then return nil for #min and #max as this is also the standard behavior for Enumerable types. The only method that's non-trivial to implement would be #deviation_of (but when I think about it I'm not sure whether we need that method in TimeFrame at all).

@bstoecker
Copy link

we already used the #deviation_of method within one of our apps. in case of an empty time_frame I would suggest just to raise an error, since the reponse would be undefined. to implement an empty timeframe, we have two possibilities:

  1. the first one is to add a flag. this would be the short, but dirty solution which I don't prefer, since we would lose maybe our min/max validation.
  2. the second solution could be an empty timeframe class which behaves as expected. This kind of timeframe should usually not be initialized by a user (which would not make much sense). This is a solution which might be better. what are you thinking about that?

@jzernisch
Copy link
Contributor

@bstoecker: Not sure whether I understand what you mean with (1). I'd prefer a simple null object similar to Array or Set. This object could be held in a constant like TimeFrame::Empty or something like that, so one could reference it, but it wouldn't be constructible via .new (although it's an instance of TimeFrame, of course). I guess this is also what you mean with (2), right?
Regarding the #deviation_of method: Actually it's just the Hausdorff Distance, so we could just use its canonical extension for empty sets (distance between two empty sets is 0, distance between a non-empty set and the empty set is positive infinity).

@pderichs
Copy link
Contributor

So we need an empty TimeFrame.

I would suggest to enhance the TimeFrame class so it can deal with empty TimeFrame instances correctly, means: handling the case of min and max with nil values correctly within the class.

I would suggest to provide an constructor which can handle no params which would lead to an empty TimeFrame instance by default (we are now forcing min, max or duration to be present). So the bound checks can stay for the case if min, max or duration are present in the params hash.

I don't consider #deviation_of to be useless - but this is another topic. For now I would suggest to throw an error if #deviation_of is called for an empty TimeFrame instance (it would make no sense to call it and it would be the easiest solution).

@bstoecker
Copy link

regarding #deviation_of I am with you @jzernisch. I think this would be a good solution.

@pderichs
Copy link
Contributor

@jzernisch Can you clarify the meaning of "I'd prefer a simple null object similar to Array or Set." ? Do you mean Null Objects like https://github.com/avdi/naught or do you mean something else?

@bstoecker
Copy link

in case of an empty-timeframe object we should also consider to change the behavior of #empty?. Currently every timeframe having no interior points is empty. maybe the method #empty? should only return true, when we use our "empty frame object". the functionality of the current #empty?method can be handled within a method #interior?.

@jzernisch
Copy link
Contributor

@pderichs What I mean was that we have a TimeFrame object that represents the empty time frame in the sense that it responds to alle TimeFrame methods in a reasonable way. I'm not sure whether such an object should be constructible or not and how one would implement it. Dropping the requirement of lower and upper bounds as you proposed would be possible.
@bstoecker I agree with you, although I find a method named "interior" a bit too mathy (I wonder whether it's clear to non-math people what that means).

Does handling all this set-theoretic stuff in a TimeFrame class actually make sense? I mean we're talking about empty sets, intersections, unions and so on -- nothing of which is specific to time. Wouldn't it make sense to outsource that logic into a separate gem (time_frame would then be depending on)?

@jzernisch jzernisch mentioned this issue Jun 25, 2014
@jzernisch jzernisch self-assigned this Jul 16, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants