Skip to content

Code cleanup and some optimizations#1911

Merged
tritao merged 5 commits intomono:mainfrom
duckdoom5:misc/code-cleanup-optimize
Feb 12, 2025
Merged

Code cleanup and some optimizations#1911
tritao merged 5 commits intomono:mainfrom
duckdoom5:misc/code-cleanup-optimize

Conversation

@duckdoom5
Copy link
Copy Markdown
Contributor

Sorry for the large amount of changes. Though I made sure this PR only contains simple changes and some fixes/optimizations (like adding const& and using TryGetValue etc.) so this commit should have no breaking changes.

One change that could be breaking is the removal of the using namespace CppSharp::CppParser::AST from one of the header files, but I really don't think it should be there since it then leaks into other translation units.

@duckdoom5
Copy link
Copy Markdown
Contributor Author

duckdoom5 commented Feb 8, 2025

Also, on that same note. Would you mind making a short list of code style preferences for this project? (please make one for C++ code and one for C#).

For example:

Naming C++:

public variable: pascalCase;
private variable: _pascalCase;
Event variable: OnTitleCase;
Function: TitleCase;
Type: class/struct TitleCase
Typedef: using MyTypeT = ExistingType;
Concept: concept IsTitleCase[T] (suffix with T if Type check)
Template type: template <[Concept] class [TypeName]T>
enum: enum class TitleCase
enum value: TitleCase;
constant: TitleCase;
namespace: namespace Word::Word::Word (prefer short names)

braces/indent sample:

namespace My::Namespace 
{
    void MyFunction()
    {
        if (!expression) // One line allowed
            return;
        
        switch(i) {
            case 1:
                // branch A
                return;
            case 2:
            {
                // branch C
                return;
            }
        }
    }
}

Class definition order:
public:
typedefs
constants
constructors
operators
functions
vars
protected:
private:

same order as public

@tritao
Copy link
Copy Markdown
Collaborator

tritao commented Feb 8, 2025

Also, on that same note. Would you mind making a short list of code style preferences for this project? (please make one for C++ code and one for C#).

For example:

Naming C++:

public variable: pascalCase; private variable: _pascalCase; Event variable: OnTitleCase; Function: TitleCase; Type: class/struct TitleCase Typedef: using MyTypeT = ExistingType; Concept: concept IsTitleCase[T] (suffix with T if Type check) Template type: template <[Concept] class [TypeName]T> enum: enum class TitleCase enum value: TitleCase; constant: TitleCase; namespace: namespace Word::Word::Word (prefer short names)

braces/indent sample:

namespace My::Namespace 
{
    void MyFunction()
    {
        if (!expression) // One line allowed
            return;
        
        switch(i) {
            case 1:
                // branch A
                return;
            case 2:
            {
                // branch C
                return;
            }
        }
    }
}

Class definition order: public: typedefs constants constructors operators functions vars protected: private: same order as public

I am not too picky about it to be honest, we should probably go with a consistency style that doesn't cause too many changes to existing code, if it can reasonably match that existing style, I think it would be good, though with Git's support for introducing ignored revisions as not to break blame, maybe changes are not that important.

What are you thinking, should we use clang-format maybe? Not sure if we should try to match LLVM/Clang coding style, or keep it more close to C# conventions maybe?

@duckdoom5
Copy link
Copy Markdown
Contributor Author

Yeah I was thinking clang-format

@duckdoom5
Copy link
Copy Markdown
Contributor Author

duckdoom5 commented Feb 8, 2025

Also, I know this doesn't compile. The reason is that there is a mismatch in the c# and CLI bindings for std::vector as you had noted before.

I was looking at implementing it as a simple type map with a getter/setter pair like this:

get {
    for (uint i = 0; i < <ProperyName>Count; ++i)
        yield return Get<ProperyName>(i);
}
set {
    Clear<ProperyName>();
    foreach (var __x in value)
        Add<ProperyName>(__x);
}

But the source generator is giving me some problems with the implementation. For example it's now generating a private field which should not be there.

@duckdoom5 duckdoom5 force-pushed the misc/code-cleanup-optimize branch 2 times, most recently from 90d3b86 to 36ef9ad Compare February 8, 2025 22:41
@duckdoom5 duckdoom5 force-pushed the misc/code-cleanup-optimize branch from 36ef9ad to 427c9b0 Compare February 11, 2025 22:42
@duckdoom5
Copy link
Copy Markdown
Contributor Author

@tritao Got this working, so just wanted to let you know it's ready for review. (No pressure :p)

@tritao
Copy link
Copy Markdown
Collaborator

tritao commented Feb 12, 2025

Looks great, thanks for much needed cleanups 👍

@tritao tritao merged commit f0b44b4 into mono:main Feb 12, 2025
6 checks passed
@duckdoom5 duckdoom5 deleted the misc/code-cleanup-optimize branch February 12, 2025 11:50
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