nus-cs2030 / 2021-s2

2 stars 2 forks source link

Cyclic Dependencies between Done, Rest and Serve #266

Open hua-fong opened 3 years ago

hua-fong commented 3 years ago

Summary

Code runs the test cases but fails in #CodeCrunch.

Description

Cyclic dependencies between Done, Rest and Serve. The issue is that my ServeEvent produces a new DoneEvent. Then DoneEvent might produce a new ServeEvent (if there are customers in queue) or a new RestEvent (if there is rest time allocated). RestEvent will produce new ServeEvents after the rest period is over.

Hence ServeEvent -> DoneEvent -> RestEvent(optional) -> ServeEvent cyclical dependency.

I am unable to solve it as any use of extra classes to produce these events would just elongate the cycle.

Does anyone know how to solve this? #

Screenshots (if any):

image

RachelCheah commented 3 years ago

I'm not sure how exactly you implemented the classes but is it possible to create an interface/abstract class that all the different events implements/extends so that there will not be any cyclic dependency?

Ryanng-zr commented 3 years ago

Hi, I think you could first try making your DoneEvent a "last event" to remove the cyclic dependency. And in your SImulation class, when polling an event from the priority queue, check whether the event is a DoneEvent, and then check whether there are customers in the queue for the particular server, and from there decide whether to return a ServeEvent or not. Hope this helps!

lohwenxin commented 3 years ago

Hi, I think you could first try making your DoneEvent a "last event" to remove the cyclic dependency. And in your SImulation class, when polling an event from the priority queue, check whether the event is a DoneEvent, and then check whether there are customers in the queue for the particular server, and from there decide whether to return a ServeEvent or not. Hope this helps!

Does this mean we are allowed to use InstanceOf in our project? I heard that it is not allowed in Code Crunch

gwajoon commented 3 years ago

Hi, I think you could first try making your DoneEvent a "last event" to remove the cyclic dependency. And in your SImulation class, when polling an event from the priority queue, check whether the event is a DoneEvent, and then check whether there are customers in the queue for the particular server, and from there decide whether to return a ServeEvent or not. Hope this helps!

Does this mean we are allowed to use InstanceOf in our project? I heard that it is not allowed in Code Crunch

Nope, instanceof is not allowed and will be flagged out by CodeCrunch. Instead, consider other methods of checking which event it is!

maoxin53 commented 3 years ago

u could try checking if new Serve need to be created in the priority queue loop of Simulation instead of checking within your Done class, also Done could always create a RestEndEvent, but with the possibility of 0 time, which would make your implementation to Server -> Done -> RestEnd -> possible new Serve

Ryanng-zr commented 3 years ago

Hi yup we are not allowed to use instanceOf but we can find other ways, such as creating methods like getEventName() to identify the events! hope it helps

RachelCheah commented 3 years ago

yeap, if you're only looking to flag a particular type of event, such as a Done Event, you can create a isDoneEvent() method that returns false in the parent event class and you'll only need to override this method in the DoneEvent class.

mingzhou128 commented 3 years ago

Hi, I think you could also try creating a new class to instantiate your events. So instead of ServeEvent producing a new DoneEvent, you can have ServeEvent return some identifier/data and use that data with a new class to instantiate your DoneEvent.

chelleadel commented 3 years ago

Hi! I think one suggestion is not to use event subclasses but rather create a shared property within Event that allows the event manager to detect the Event type. Either making an ENUM or Name attribute.

keithcharlesc commented 3 years ago

To add on to what the rest have mentioned, it might be better to make the done event the last event so that it does not keep generating! In this case, "ServeEvent -> DoneEvent -> RestEvent(optional) -> ServeEvent" will possibly generate another DoneEvent after the ServeEvent stage at the end of the chain and create a continuous cycle

whoisjustinngo commented 3 years ago

Hi! how i got around this was just to create a new serveevent from Main. so my flow (in Main) is .... -> ServeEvent -> DoneEvent -> RestEndEvent (-> represents "generates a") then once i detected that the event I poll from the pq is a RestEndEvent, I manually create a new ServeEvent in Main and enqueue it if necessary. This way DoneEvent does not loop back and depend on ServeEvent You can perhaps add static attributes to your event classes to serve as identifiers i.e. ArrivalEvent = 1, ServeEvent = 2 DoneEvent = 0 RestEvent = -1 etc and get them when you need to check if the event is of a particular type. the identifiers might also help in ordering the events in the pq