New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration file instead of using ifdef statements to control the way the app works #354

Closed
pieterclaerhout opened this Issue Nov 3, 2014 · 14 comments

Comments

Projects
None yet
6 participants
@pieterclaerhout

pieterclaerhout commented Nov 3, 2014

I'm currently cleaning up the code, and I'm tempted to get rid of the ifdef statements to control the way the app works.

They are spreaded all over the place, making it hard for people to make adjustments to the app.

I'm very tempted to change this into a config file (e.g. settings.plist which is part of the app bundle) that is then evaluated during runtime to determine how the app should work. This way, by changing the settings file, you can change it from a single-issue to a newsstand app.

What's everybody's opinion on this?

@pieterclaerhout pieterclaerhout self-assigned this Nov 3, 2014

@pieterclaerhout pieterclaerhout added this to the 4.3 milestone Nov 3, 2014

@akeilox

This comment has been minimized.

akeilox commented Nov 3, 2014

this is a great idea!

@nicholasmartin

This comment has been minimized.

nicholasmartin commented Nov 3, 2014

I like it a lot as well. Would also help define things like push notifications, google analytics and other settings that are on an app basis.

@macuarium

This comment has been minimized.

macuarium commented Nov 3, 2014

Right now, archiving the master version gives a "unterminated conditional directive" error with the ifdefs of constants.h (because they're opened twice).

On the other hand, I completely like the idea of a single variables file.

Great work.

@pieterclaerhout

This comment has been minimized.

pieterclaerhout commented Nov 4, 2014

@macuarium Are you sure you have the latest version of master? I just downloaded it and was able to archive it without any errors.

@benoitchantre

This comment has been minimized.

benoitchantre commented Nov 4, 2014

Great idea: a global config file will ease the configuration and the maintenance.

@macuarium

This comment has been minimized.

macuarium commented Nov 4, 2014

@pieterclaerhout I had just downloaded it, but may have made a mistake myself as I was juggling versions :-). I just cleared the two lines and it worked fine. Might be my bad housekeeping.

@feralbob

This comment has been minimized.

feralbob commented Nov 4, 2014

Can we just set defaults in the user defaults and then allow those to be
overridden?

On Tue, Nov 4, 2014 at 5:49 AM, Miguel Cornejo notifications@github.com
wrote:

@pieterclaerhout https://github.com/pieterclaerhout I had just
downloaded it, but may have made a mistake myself as I was juggling
versions :-). I just cleared the two lines and it worked fine. Might be my
bad housekeeping.


Reply to this email directly or view it on GitHub
#354 (comment)
.

@pieterclaerhout

This comment has been minimized.

pieterclaerhout commented Nov 4, 2014

Technically, we can, but this would mean you can only influence them from within the app.

With an extra config file, we can customize them easily on a build by build basis.

@feralbob

This comment has been minimized.

feralbob commented Nov 4, 2014

Can't we use

registerDefaults:
file:///Users/bob/Library/Developer/Shared/Documentation/DocSets/com.apple.adc.documentation.AppleiOS8.1.iOSLibrary.docset/Contents/Resources/Documents/documentation/Cocoa/Reference/Foundation/Classes/NSUserDefaults_Class/index.html#//apple_ref/occ/instm/NSUserDefaults/registerDefaults:

With a custom file - then we don't need to add any additional stuff to
query our own settings - we can just query UserDefaults

B

On Tue, Nov 4, 2014 at 10:05 AM, Pieter Claerhout notifications@github.com
wrote:

Technically, we can, but this would mean you can only influence them from
within the app.

With an extra config file, we can customize them easily on a build by
build basis.


Reply to this email directly or view it on GitHub
#354 (comment)
.

@pieterclaerhout

This comment has been minimized.

pieterclaerhout commented Nov 4, 2014

I don't think using registerDefaults is a good idea for this. If you start changing settings and already had the app installed, you would need to flush them out before you can use the new values.

I'm thinking about creating a singleton that reads in the plist file during it's initialization and assign the properties to member variables. These can be read only as they don't need to be changed during runtime anyway.

This has the advantage that rebuilding the app immediately picks up the new values.

You can then even do tricks like building the app once, updating the plist and resign to change the way the app works.

@feralbob

This comment has been minimized.

feralbob commented Nov 4, 2014

The registerDefaults are cleared each run - from the docs

The contents of the registration domain are not written to disk; you need
to call this method each time your application starts. You can place a
plist file in the application's Resources directory and call
registerDefaults: with the contents that you read in from that file.

So as I understand it - we get the same benefit without having to write the
singleton

On Tue, Nov 4, 2014 at 10:51 AM, Pieter Claerhout notifications@github.com
wrote:

I don't think using registerDefaults is a good idea for this. If you start
changing settings and already had the app installed, you would need to
flush them out before you can use the new values.

I'm thinking about creating a singleton that reads in the plist file
during it's initialization and assign the properties to member variables.
These can be read only as they don't need to be changed during runtime
anyway.

This has the advantage that rebuilding the app immediately picks up the
new values.

You can then even do tricks like building the app once, updating the plist
and resign to change the way the app works.


Reply to this email directly or view it on GitHub
#354 (comment)
.

@pieterclaerhout

This comment has been minimized.

pieterclaerhout commented Nov 4, 2014

Ok, point taken. Nevertheless, we will have to create a category that can cast the values (e.g. hex colors to UIColor objects) to their proper classes...

@pieterclaerhout

This comment has been minimized.

pieterclaerhout commented Nov 4, 2014

And we also need to write code that uses the proper default values for everything that is not specified in the plist file. My gut feeling says that writing a custom class isn't that much more work, and makes it easier to change later on.

@feralbob

This comment has been minimized.

feralbob commented Nov 4, 2014

Not sure I'm following you Pieter - can't we just set the defaults in the
settings.plist we ship - then register those defaults? As for creating a
category - what's wrong with just using objectForKey?

On Tue, Nov 4, 2014 at 11:26 AM, Pieter Claerhout notifications@github.com
wrote:

And we also need to write code that uses the proper default values for
everything that is not specified in the plist file. My gut feeling says
that writing a custom class isn't that much more work, and makes it easier
to change later on.


Reply to this email directly or view it on GitHub
#354 (comment)
.

pieterclaerhout added a commit that referenced this issue Nov 5, 2014

pieterclaerhout added a commit that referenced this issue Nov 5, 2014

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