Skip to content

Use consistent coding style across all packages #354

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

Closed
swift-kim opened this issue Apr 12, 2022 · 5 comments
Closed

Use consistent coding style across all packages #354

swift-kim opened this issue Apr 12, 2022 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@swift-kim
Copy link
Member

swift-kim commented Apr 12, 2022

This issue is for tracking the code refactoring status of this repo. The main goal is to improve

  • Overall code quality
  • Readability and maintainability
  • Consistency

of all C++ code.

Here are some general rules for styling the code:

  • Everything from the Google C++ Style Guide.
  • Structure:
    • If the plugin class (which defines the RegisterWithRegistrar method) is too large (over 150 lines), consider breaking it into multiple classes.
      • The classes should have a clean design so that their roles can be easily known from their signatures. Ideally, the plugin class should be the only one that interacts with the underlying platform channel (processes method arguments and generates results), and other classes should not touch any Flutter API (flutter/*.h).
    • Enclose the plugin class within an anonymous namespace.
    • If you're writing a Tizen C API wrapper, avoid exposing Tizen-defined types (including enums) in its public interface.
    • Minimize use of output parameters when writing a function that returns a value. Consider using the GetLastError pattern (example) or defining your own result type such as TizenResult.
      • Consider making use of C++ exceptions if they make the code simpler and easier to understand (e.g. multiple platform API invocations per single method call). See this comment for other available options.
    • Consider creating an explicit stream handler class that extends flutter::StreamHandler if the plugin makes use of event channels. (example)
  • Logging (log.h):
    • Avoid unnecessary logging. Minimize use of debug logs (LOG_DEBUG) in the final code.
    • Every log should provide useful information. Any information that is already known to the caller (through a PlatformException object) is redundant. Instead, provide some additional context about the error so that the developer can easily debug the underlying issue.
  • Method call handling:
    • Consider using GetValueFromEncodableMap to extract arguments from an EncodableMap.
    • Prefer == over .compare() when comparing strings.

      const auto &method_name = method_call.method_name();
      
      if (method_name == "wifiName") {
        ...
      }
    • An error code (the first argument of result->Error()) should be either:
      • A return code (number converted to a string)
      • Invalid argument
      • Invalid operation
      • Operation failed/cancelled
      • Permission denied
      • Internal error
      • etc.
    • Do not skip the second argument (error message) of result->Error() whenever possible.
  • Variable naming convention:
    • Use a local variable ret to store the return code of a Tizen API call.
    • Use a local variable self or plugin to store a pointer to the current class.
  • Other recommendations:
    • Remove any unused includes (especially standard library headers).
    • Prefer if (value) over if (value != nullptr).
    • Consider inlining a C-style callback using a lambda expression if it makes the code cleaner.
    • Prefer fixed-width integer types (int32_t, int64_t) over plain old integer types (int, long long). This is not a strict requirement, and there are places where a plain int fits best. Also, you should avoid use of unsigned integer types.

I will start with 1st-party plugins and plus package implementations (there are already some PRs open). If you want to contribute to this project, you may start with 3rd-party plugins and Tizen-only plugins, or plugins authored by you if you have any.

Any comments are welcome. Please let me know if anything in the above is unclear.

@swift-kim swift-kim added the enhancement New feature or request label Apr 12, 2022
@bbrto21
Copy link
Contributor

bbrto21 commented Apr 12, 2022

That's a great suggestion. I think you've organized the things that we have to do for clean code, it's very clear.
I will also try to contribute to this issue as often as possible.

@bbrto21
Copy link
Contributor

bbrto21 commented May 10, 2022

@JSUYA Could you take a look at the geolocator(or whatever you want)?

@bbrto21
Copy link
Contributor

bbrto21 commented May 11, 2022

@Swanseo0 Hello, congratulation to join us. 🎉

I think this issue is good to learn about the work process of flutter-tizen. if you don't mind, please take a look at this issue and try to contribute. there are many other packages, but I recommend you to refactoring tizen_app_manager.
It's up to you to contribute to this, so don't feel pressured and you are free to participate in any open issues.

P.S don't forget to read this guide.

@JSUYA
Copy link
Member

JSUYA commented May 12, 2022

@JSUYA Could you take a look at the geolocator(or whatever you want)?

@bbrto21 Thanks for the suggestion. I will check

bbrto21 added a commit to bbrto21/plugins that referenced this issue Oct 4, 2022
This patch includes:
* Simplify WearableRotaryPlugin Implementation.
* Use derived class from flutter::StreamHandler to set stream handlers.
* Use derived class from flutter::StreamHandlerError to hold references safely.

This change contribute flutter-tizen#354

Signed-off-by: Boram Bae <[email protected]>
bbrto21 added a commit to bbrto21/plugins that referenced this issue Oct 5, 2022
This patch includes:
* Simplify WearableRotaryPlugin Implementation.
* Use derived class from flutter::StreamHandler to set stream handlers.
* Use derived class from flutter::StreamHandlerError to hold references safely.

This change contribute flutter-tizen#354

Signed-off-by: Boram Bae <[email protected]>
bbrto21 added a commit that referenced this issue Oct 5, 2022
[wearable_rotary] Refactor the c++ code

This patch includes:
* Simplify WearableRotaryPlugin Implementation.
* Use derived class from flutter::StreamHandler to set stream handlers.
* Use derived class from flutter::StreamHandlerError to hold references safely.

This change contribute #354

Signed-off-by: Boram Bae <[email protected]>
@swift-kim
Copy link
Member Author

swift-kim commented Jan 6, 2023

Remaining packages to be worked on:

  • camera_tizen
  • messageport_tizen
  • sqflite_tizen
  • tizen_audio_manager
  • tizen_rpc_port
  • wakelock_tizen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants