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

Reduce complexity of headers with d-pointers and "only declaration" rule #2564

Closed
lisitsyn opened this issue Oct 25, 2014 · 11 comments
Closed
Labels

Comments

@lisitsyn
Copy link
Member

I propose to change:

class CClass
{
public:
  void method();
  int get_field() const
  {
    return field;
  }
private:
  int m_field;
};

// .cpp

void CClass::method()
{
   .. m_field ..
}

to a better looking (and faster compiling) variant:

class CClass 
{
public:
    void method();
    int get_field() const;
private:
    class Self;
    Unique<Self> self;
};

// .cpp

class CClass::Self
{
  int m_field;
};

CClass::get_field()
{
  return self->m_field;
}

CClass::method()
{
   .. get_field(); ..
}

It would involve massive refactoring so should be done once and fast. One thing that comes for free - in our code we will not use member variables w/o getters anymore (which is considered good). Another advantage is that very likely SWIG will compile much faster.

I think possible approach involves writing some script instead of doing it manually.

@karlnapf
Copy link
Member

I love this idea, but SWIG will not compile faster, see https://github.com/shogun-toolbox/shogun/wiki/SWIG-issues

Many other things will improve through this though.

@iglesias
Copy link
Collaborator

iglesias commented Feb 3, 2015

@lisitsyn, why is it good not to use member variables w/o getters (and setters too, I guess) within the class where they are members?

@lisitsyn
Copy link
Member Author

lisitsyn commented Feb 5, 2015

@iglesias I'd say it is more flexible

@iglesias
Copy link
Collaborator

iglesias commented Feb 5, 2015

I am not sure what flexible is here. Can you put an example, @lisitsyn? :-)

@karlnapf
Copy link
Member

karlnapf commented Feb 8, 2015

why does this compile faster?

@lisitsyn
Copy link
Member Author

lisitsyn commented Feb 8, 2015

Okay guys a few cases that come into my mind:

  1. If we go with generic get/set without specific getters for field adding a new field is free - you just recompile the .cpp file (@iglesias).
  2. In the same scenario headers are binary compatible with various versions of shogun (@iglesias).
  3. We can remove all mentions of std vector/list/.. from headers because they are only in .cpp files so it will be faster (@karlnapf).
  4. We can probably use simpler saving/loading that just works with that subobject (declared in .cpp) but not the object (declared in .h) - that's a thing I am thinking about yet.

@karlnapf
Copy link
Member

karlnapf commented Feb 8, 2015

+1 from my side to all this

@iglesias
Copy link
Collaborator

It sounds pretty awesome!

@iglesias
Copy link
Collaborator

Thanks a lot for the nice description, @lisitsyn :-)

@stale
Copy link

stale bot commented Feb 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 26, 2020
@vigsterkr
Copy link
Member

obsolete

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

No branches or pull requests

4 participants