C# periodssssss

Question

I got the following assignment:

Returns true if the given appointment can be added to the list of appointments without overlapping
any other appointments.
name="appointments">The list of current appointments
name="appointment">The new appointment for which to check if it fits within the other appointments
True if the new appointment fits between the current appointments: "9-10", "11-12", "15-16"

My current code is:

public bool IsAvailable(Appointment[] appointments, Appointment appointment)
{                
    bool overlap = true;

    foreach (var afspraak in appointments)
    {
        overlap = (afspraak.Start >= appointment.End || appointment.Start <= afspraak.End);
    }

    return overlap;
}

This code passes 10 out of the 18 unit tests:

Should return True:
        [TestCase("8-9")]       passed
        [TestCase("8-8:30")]    passed
        [TestCase("10-11")]     passed
        [TestCase("10:45-11")]  passed
        [TestCase("12-15")]     passed
        [TestCase("12-13")]     passed
        [TestCase("13-14")]     passed
        [TestCase("14-15")]     passed
        [TestCase("16-17")]     passed
        [TestCase("16:30-17")]  failed

Should return False:
        [TestCase("7-8")]       failed
        [TestCase("8-9:30")]    failed
        [TestCase("9:30-9:45")] failed
        [TestCase("9:30-11")]   failed
        [TestCase("8-10:30")]   failed
        [TestCase("17-18")]     passed
        [TestCase("12-18")]     failed
        [TestCase("11:30-12:30")] failed

I just can't really figure out what I'm doing wrong, anyone seeing what I'm doing wrong?

UPDATE 2: The problem is that only now the "should return false unit tests" pass :(

------

You aren't breaking out of the for-loop so only the last afspraak in appointments is influencing the overlap value. — Slate, May 14 '21 at 11:03

DekuDesu · Answer 1 · 2021-05-14T11:47:11.557

2

With your current method the bool overlap is ONLY set to whatever the last comparison result was.

Consider breaking/returning as soon as you detect that the appointment can't be added without overlapping.

public bool IsAvailable(Appointment[] appointments, Appointment appointment)
{                
    foreach (var afspraak in appointments)
    {
        if((afspraak.Start >= appointment.End && appointment.End <= afspraak.Start) is false)
        {
            // return here, if the appointment overlaps, no need to continue checking
            return false;
        }
    }

    return true;
}

edited May 14 '21 at 11:47

answered May 14 '21 at 11:07

DekuDesu

2,224
1
5
19

1

Well, it will save precious computing time too! Apart from that, you don't need `is false` or `==false` when you're dealing with boolean expressions. – XouDo May 14 '21 at 11:22
Absolutely! `is false` is not required for conditional boolean statements. However, whether or not using the bang symbol (!) reduces readability is up to debate. I would recommend following the teachers required code style and design guidelines. It should also be noted the compiler reduces this comparison to the same IL. – DekuDesu May 14 '21 at 11:26
First of all, thank you for answering! I checked out your code and you are right that when it faces a false it should not continue to check on the other overlaps! But if put your changes into my code it will only pass on the returns true unit tests. See update2 – maate100 May 14 '21 at 11:37
I went ahead and updated it for you, try that out. I modified the conditional check. – DekuDesu May 14 '21 at 11:50
See update 2 for my code btw :) – maate100 May 14 '21 at 12:27

C# periodssssss

1 Answers1