Skip to content

#306 Used NSUbiquitousKeyValueStore.default #310

Open
VigneshPalanisamy wants to merge 2 commits intomasterfrom
WS-306
Open

#306 Used NSUbiquitousKeyValueStore.default #310
VigneshPalanisamy wants to merge 2 commits intomasterfrom
WS-306

Conversation

@VigneshPalanisamy
Copy link
Contributor

to store the preferences in iCloud.

self.localPreferences.set("", forKey: "presentationSlide")
self.localPreferences.set(0, forKey: "presentationSlideNumber")
self.localPreferences.set("", forKey: "presentationAuthor")
self.preferences.synchronize()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we synchronize local preferences?

if !preferences.dictionaryRepresentation().keys.contains("defaultDatabase") {
self.preferences.set(true, forKey: "defaultDatabase")
self.preferences.synchronize()
if !localPreferences.dictionaryRepresentation().keys.contains("defaultDatabase") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider grouping all local preferences together. If possible, extract them to a method.

self.preferences.synchronize()

if !preferences.dictionaryRepresentation().keys.contains("displayRomanised") {
if !preferences.dictionaryRepresentation.keys.contains("displayRomanised") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider having two methods to group this logic.
configureDefaultPreferences
configureDefaultLocalPreferences

secondScreenView.songNameLabel.font = UIFont.systemFont(ofSize: CGFloat(fontSize) / 3)
secondScreenView.authorLabel.font = UIFont.systemFont(ofSize: CGFloat(fontSize) / 3)
}
let fontColor = self.preferences.string(forKey: "presentationEnglishFontColor")!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using if let here.

nextButton.isHidden = self.verseOrder.count <= 1
self.preferences.setValue(authorName, forKeyPath: "presentationAuthor")
self.preferences.setValue(songName, forKeyPath: "presentationSongName")
self.preferences.set(authorName, forKey: "presentationAuthor")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be set in local preferences?

var presentationIndex = 0
var comment: String = ""
fileprivate let preferences = UserDefaults.standard
fileprivate let preferences = NSUbiquitousKeyValueStore.default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need both local and icloud preferences in this class.

self.presentationData.setupScreen()
let activeSection = preferences.integer(forKey: "presentationSlideNumber")
indexPath = IndexPath(row: 0, section: activeSection)
if let activeSection = preferences.object(forKey: "presentationSlideNumber") as? Int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be from local?

cell.textLabel!.numberOfLines = 0
let fontSize = self.preferences.integer(forKey: "fontSize")
cell.textLabel?.font = UIFont.systemFont(ofSize: CGFloat(fontSize))
if let fontSize = self.preferences.object(forKey: "fontSize") as? Int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is repeating in many places. How about having a PreferenceService with a method getFontSize?
This service can encapsulate whether something should be stored/retrieved in/from local or iCloud preference. It would be completely transparent for all the other classes. They need not care where it is stored.

let version = getVersion()
if preferences.dictionaryRepresentation().keys.contains("version") {
if !(preferences.string(forKey: "version")?.equalsIgnoreCase(version))! {
if let appVersion = preferencesService.object(forKey: "version", local: true) as? String {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider having a method named getVersion in preferencesService.

@@ -82,14 +81,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
for i in 0..<favSongs.count {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving all the import favourites related logic to a separate method in this class.

self.preferences.setValue(0, forKey: "presentationSlideNumber")
self.preferences.setValue("", forKey: "presentationAuthor")
self.preferences.synchronize()
preferencesService.set("", forKey: "import.status", local: true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving all local settings to a separate method in this class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants