Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

    To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  4. You must use dependency injection. For example even things like EKEventStore() should be injected.

    final class iCal: Events {
     private let eventStore: EKEventStore
     init(with eventStore: EKEventStore = EKEventStore()) {
     self.eventStore = eventStore
     }
    }
    
  5. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  6. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

  7. You should make class final when don't want to allow subclassing it (even if when you are not sure for now). If you do need subclassing later, you can remove final keyword.

To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  1. You must use dependency injection. For example even things like EKEventStore() should be injected.

    final class iCal: Events {
     private let eventStore: EKEventStore
     init(with eventStore: EKEventStore = EKEventStore()) {
     self.eventStore = eventStore
     }
    }
    
  2. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  3. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

  4. You should make class final when don't want to allow subclassing it (even if when you are not sure for now). If you do need subclassing later, you can remove final keyword.

All the best!

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  1. You must use dependency injection. For example even things like EKEventStore() should be injected.

    final class iCal: Events {
     private let eventStore: EKEventStore
     init(with eventStore: EKEventStore = EKEventStore()) {
     self.eventStore = eventStore
     }
    }
    
  2. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  3. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

  4. You should make class final when don't want to allow subclassing it (even if when you are not sure for now). If you do need subclassing later, you can remove final keyword.

All the best!

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

    To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  4. You must use dependency injection. For example even things like EKEventStore() should be injected.

    final class iCal: Events {
     private let eventStore: EKEventStore
     init(with eventStore: EKEventStore = EKEventStore()) {
     self.eventStore = eventStore
     }
    }
    
  5. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  6. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

  7. You should make class final when don't want to allow subclassing it (even if when you are not sure for now). If you do need subclassing later, you can remove final keyword.

All the best!

Bounty Awarded with 50 reputation awarded by Community Bot
added 222 characters in body
Source Link
Anand
  • 201
  • 1
  • 3

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  1. You must use dependency injection. For example even things like EKEventStore() should be injected.

    final class iCal: Events {
     private let eventStore: EKEventStore
     init(with eventStore: EKEventStore = EKEventStore()) {
     self.eventStore = eventStore
     }
    }
    
  2. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  3. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

  4. You should make class final when don't want to allow subclassing it (even if when you are not sure for now). If you do need subclassing later, you can remove final keyword.

All the best!

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  1. You must use dependency injection. For example even things like EKEventStore() should be injected.

  2. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  3. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

All the best!

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  1. You must use dependency injection. For example even things like EKEventStore() should be injected.

    final class iCal: Events {
     private let eventStore: EKEventStore
     init(with eventStore: EKEventStore = EKEventStore()) {
     self.eventStore = eventStore
     }
    }
    
  2. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  3. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

  4. You should make class final when don't want to allow subclassing it (even if when you are not sure for now). If you do need subclassing later, you can remove final keyword.

All the best!

Source Link
Anand
  • 201
  • 1
  • 3

Please consider follow recommendations:

  1. Make habit of extracting each struct/class/enum in separate files which brings more readability and clarity. Code looks less cluttered.

  2. Please try getting used to of not using semicolons :)

  3. You have defined Events as a class with empty implementations of functions. Are you sure you want to do that, I think it could be converted as a protocol instead

To be able to inject different type of calendar APIs, you should define your own protocol and make concrete implementations for each calendar (Adapter Pattern).

  1. You must use dependency injection. For example even things like EKEventStore() should be injected.

  2. Using force unwrap is a bad practice, you should use guard let or if let to unwrap the optional value and only then use that unwrapped value for whatever purpose you want. By using forced unwrapping your app can crashes unexpectedly where it will find a nil value inside any optional variable.

  3. Please make use of Swiftlint to make sure that your code is following standard convention. It will force you to think swift way :)

All the best!

default

AltStyle によって変換されたページ (->オリジナル) /