Static constructors generally do lead to the problems you have encountered, where the implementation tends to be procedural, and complexity increases as the constructed classes become more intricate.
This can be made more manageable by using a builder pattern, which is implemented as a class that exists for the purpose of constructing the final class. This allows you to maintain an object-oriented approach.
The builder class can be used in the static parse
function, so the interface can stay the same if needed.
Ideally, the parse
function should be implemented as a convenience initialiser since it is constructing an object instance.
To answer your questions:
Should local variables such as allInput
that are passed in to more than one function be instance variables?
Yes. These should be instance variables of the builder. See the example below.
Should functions like 'determineMeetingFormat' and 'parseParticipants' be called directly into the Meeting constructor or be assigned to local constants that clearly defines what's being returned?
Yes. Local constants/variables are self documenting to some degree and hence improve readability. They also give you an easy reference to trace during debugging if needed.
Would it be better to make location
and description
optional Strings rather than empty strings?
Yes. By making these optionals the compiler can help you in those places where you might need to check if the string exists. The alternative is to use isEmpty
which you can only see at runtime / unit tests. Compile time checking gives more immediate results.
Is making all my functions and instance variables static the best way to make this behave like a static class?
Not always. Static methods behave like singletons in many ways, which has many disadvantages, such as being hard to test and debug. They are often provided as conveniences which delegate to an equivalent instance method. You are essentially creating a constructor, which would be better implemented as a convenience initializer (i.e. init
).
Example 1:
The example below shows one way that the builder pattern could be implemented:
Builder class
class MeetingParser {
let input: String
// Instance variables for the parsed values.
// These can be optionals for more safety.
var parsedHostCodes: [HostCode]!
var parsedPassCodes: [PassCode]!
var parsedPhoneNumbers: [PhoneNumber]!
var meetingFormat: MeetingFormat!
var participants: [Participant]!
var isAtt: Bool
var host: String!
init(input: String) {
self.input = input
}
func parse() {
// The functions below would access the instance variables.
parsedHostCodes = parseHostCode()
parsedPasscodes = parseParticipantCode()
parsedPhoneNumbers = parsePhoneNumber()
meetingFormat = determineMeetingFormat()
participants = parseParticipants()
isAtt = input.contains(pattern: attRegex)
host = retrieveHost()
}
}
Meeting convenience initialiser
extension Meeting {
init(event: EKEvent) {
let allInput = "\(event.title)\n\(event.location)\n\(event.description)"
let parse = MeetingParse(input: allInput)
parser.parse()
self.UUID = event.eventIdentifier
self.title = event.title
self.description = event.description
self.location = event.location
self.host = parser.host
self.startTime = event.startDate
self.endTime = event.endDate
self.allday = event.isAllDay
self.passcodes = parser.parsedPasscodes
self.hostcodes = parser.parsedHostCodes
self.phoneNumbers = parser.parsedPhoneNumbers
self.attendees = parser.participants
self.isAtt = parser.isAtt
self.meetingFormat = parser.meetingFormat
}
}
Example 2
Below is a second example, also using a builder class, although this one assumes the properties of the Meeting
class are implicit optionals, which simplifies the construction to some extent. This comes at the cost of reducing safety as it potentially allows nils to be unwrapped which would result in a runtime crash.
class MeetingParser {
let event: EKEvent
let meeting: Meeting
init(event: EKEvent, meeting: Meeting) {
self.event = event
self.meeting = meeting
}
func build() {
meeting.UUID = event.eventIdentifier
meeting.title = event.title
meeting.description = event.description
meeting.location = event.location
meeting.host = retrieveHost()
meeting.startTime = event.startDate
meeting.endTime = event.endDate
meeting.allday = event.isAllDay
// Class methods re-use the values in meeting.
// Could instead use instance variables to hold intermediate values.
meeting.parsedHostCodes = parseHostCode()
meeting.parsedPasscodes = parseParticipantCode()
meeting.parsedPhoneNumbers = parsePhoneNumber()
meeting.meetingFormat = determineMeetingFormat()
meeting.participants = parseParticipants()
meeting.isAtt = input.contains(pattern: attRegex)
}
}
extension Meeting {
// Meeting properties are defined as simplicity unwrapped optionals:
// var title: String! = nil
init(event: EKEvent) {
let builder = MeetingBuilder(event: event, meeting: self)
builder.build()
}
}
Example usage:
let meeting = Meeting(event: event)
Other advice:
Try to reduce the overall size of the Meeting
class to simplify the parser. Try to encapsulate related data into smaller classes. As an example, group the time and date information into an object to the properties for startTime
, endTime
and isAllDay
into a single property:
enum MeetingTime {
case allDay
case range(start: Date, end: Date)
}
class Meeting {
// ... other properties
var time: MeetingTime?
// ... other properties
}
let meeting: Meeting
meeting.time = .allDay // All day meeting.
meeting.time = .range(start: .now, end: .now) // Meeting between 2 times.
//Regular expressions (not actually named like this)
? Is this real working code? \$\endgroup\$parse
function reviewed. I'll remove the class details to make this more clear. \$\endgroup\$