Skip to content
This repository has been archived by the owner on May 5, 2020. It is now read-only.

Update Strings.swift to group strings in enums and update accessors throughout the app. #222

Open
4 tasks
SIMRAN1 opened this issue Feb 28, 2018 · 9 comments · May be fixed by #356
Open
4 tasks

Update Strings.swift to group strings in enums and update accessors throughout the app. #222

SIMRAN1 opened this issue Feb 28, 2018 · 9 comments · May be fixed by #356

Comments

@SIMRAN1
Copy link
Contributor

SIMRAN1 commented Feb 28, 2018 •
edited by justkd
Loading

Description

As a developer,
I need to organize the constants in Strings.swift into appropriate enums,
so that the data can be more easily identified and accessed throughout the app via code completion.

Example:

enum AlertStrings: String {
        case newGameAlert = "new game alert text"
        case scenarioCompletedWarning = "scenario already completed warning text"

        etc.
}

Acceptance Criteria

Update [Required]

  • Organize string constants in Strings.swift into appropriate enumerable collections.
  • Update accessors throughout the app to use the enums.

Definition of Done

  • All of the required items are completed.
  • Approval by 1 mentor.

Estimation

2 hours
@SIMRAN1
Copy link
Contributor Author

SIMRAN1 commented Mar 1, 2018

@sunjunkie should I work on this issue?

@sunjunkie
Copy link
Contributor

Can you enhance the issue description according to the template? As a [USER], I need [TO DO THIS], so that I can [ACCOMPLISH THAT]. E.g. As a developer, I need to have all constants in the same file so that I can locate or edit these constants easily.

You can work on this but, please explain (before you edit your code) which constants you want to extract and how you will organize these constants.

@SIMRAN1
Copy link
Contributor Author

SIMRAN1 commented Mar 7, 2018 •
edited
Loading

okk @sunjunkie I will open issue according to template and send pr for it

@SIMRAN1
Copy link
Contributor Author

SIMRAN1 commented Mar 7, 2018

@sunjunkie before preceeding to work on this issue I would ask one suggestion as there are so many constants per file..How should I classify them like moving integers into Integers.swift and float values into Float.swift or moving all constants in constants.swift??

@SIMRAN1 SIMRAN1 mentioned this issue Mar 13, 2018
Closed
10 tasks
@SIMRAN1 SIMRAN1 mentioned this issue Apr 20, 2018
Closed
10 tasks
@justkd
Copy link
Contributor

justkd commented Jul 27, 2018

I'm going to update this issue to better represent the current state of the app.

I think the current level of abstraction (as far as cleaning up and aggregating re-usable strings) is fine now, but I also think they could be organized better in Strings.swift.

I suggest we update the strings to be grouped in enums, so it's easier to find and identify them throughout the app via code completion.

@justkd justkd changed the title Moving Constants to a seprate file Update Strings.swift to group strings in enums and update accessors throughout the app. Jul 27, 2018
@sunjunkie
Copy link
Contributor

Yes, I agree. See also my comments on PR #238.

@vatsalkul
Copy link
Member

Hey @sunjunkie I also think that enum would be good. I have gone through the code base and found that all Strings in Utilities/Strings.swift can be classified into some categories. like
CustomError, CustomWarning, CustomImage, CustomMessage, CustomeLabelText.
Let me know if I can work on this issue

@justkd
Copy link
Contributor

justkd commented Feb 29, 2020

Hi @vatsalkul , I think this one is fairly safe to go ahead and work on. If you make the changes as proposed and submit a PR, we can review and test it and discuss any further changes that may be necessary.

@vatsalkul vatsalkul linked a pull request Feb 29, 2020 that will close this issue
Open
3 tasks
@vatsalkul
Copy link
Member

@justkd Done! Hope you will like this approach.😇

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@sunjunkie @SIMRAN1 @justkd @vatsalkul