Skip to content
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

#define constants declared in .m should not be made global #196

Open
Ibrahimhass opened this issue Jan 26, 2022 · 4 comments
Open

#define constants declared in .m should not be made global #196

Ibrahimhass opened this issue Jan 26, 2022 · 4 comments

Comments

@Ibrahimhass
Copy link
Member

Variable declared in the .m file like in these files should not be converted to global variables.
Original Source Code
X509.m.zip
Converted Code
X509.swift.zip

@alex-swiftify
Copy link
Member

alex-swiftify commented Jan 27, 2022

@Ibrahimhass

  1. In your particular sample, even though these #define macros are defined in the .m file, they are outside of the #implementation section. In order to make them non-global, we need to include them in a class, but I cannot see a pattern or reasoning to make them members of the X509 class. Nor do I see enough reasoning to declare a custom class e.g. PrivateVariables for these variables.

  2. Even if these macros were declared inside of the #implementation section, they are allowed to be used at least from anywhere in the source file.
    This leads to the only possible suggestion - to declare them as fileprivate variables.
    Is that something that you have in mind?

IMO they still have to be refactored (casing should be changed, and variables are likely to be moved to a particular class), thus I don't see a lot of benefit of adding a fileprivate modifier. Do you?

  1. What catches my eyes in your sample is that header comments and global variables appear at the bottom of the file, which I could not reproduce by converting just the .m file. Let me know how did you get such results.

@alex-swiftify
Copy link
Member

@Ibrahimhass Please review and reply when you have time (don't spend too much time on this though).

@Ibrahimhass
Copy link
Member Author

@alex-swiftify

Even if these macros were declared inside of the #implementation section, they are allowed to be used at least from anywhere in the source file.
This leads to the only possible suggestion - to declare them as fileprivate variables.
Is that something that you have in mind?

Yes, I suggest that we add the fileprivate modifier so that the names do not collide if the same variables are used in any other class too as a private variable.

What catches my eyes in your sample is that header comments and global variables appear at the bottom of the file, which I could not reproduce by converting just the .m file. Let me know how did you get such results.

I am also not able to replicate this behavior now. I tried using the Project Converter and the Xcode Extension.

@alex-swiftify
Copy link
Member

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

No branches or pull requests

2 participants