1. Bad smells in the existing code

1.1 Bad smell definitions

 

Following points indicates the bad smells across the provided code

 

 

• ship1 should be defined as singleton object in UBCruises class as it doesn't make any sense in defining the object as non-static because if UBCruises() constructor is called more than once in Main() method, then again new ship object is created and will differ.
• No need of overriding default constructor in UBCruises.
• No usage of Properties. Instead of public variables, properties should be implemented.
• As the ship1 is singleton object, we can call setupShip method in a static constructor in UBCruises, so that only once for the execution it will be called by making setupShip() method too as static.
• Instead of using ArrayList can use a generic collection List<> for ensuring type safety when using inbuilt comparison methods like select and where etc.,
• User-defined exceptions are nowhere defined. For example, we can define a user-defined exception like CabinNotAvailableException and throw that if the Available() method returns false and can implement try/catch mechanism.

Implement IDisposable for UBCruises and override Dispose() method and clear all the collection objects in that and call it in main by implementing try/finally in the Main()