New issue
Advanced search Search tips

Issue 701124 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug

Blocking:
issue 701095



Sign in to add a comment

Eliminate multiple inheritance style guide violation for ReadingListWebStateObserver

Project Member Reported by kkhorimoto@chromium.org, Mar 13 2017

Issue description

Multiple inheritance is prohibited by the style guide unless only one superclass has an implementation.  All other superclasses must be pure interfaces with an "Interface" suffix: https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance
 
Owner: olivierrobin@chromium.org
Status: Started (was: Untriaged)
To be precise, it is not a style guide violation as it is listed as an explicit exception in the chromium style guide.
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md

Multiple inheritance

Multiple inheritance and virtual inheritance are permitted in Chromium code, but discouraged (beyond the “interface” style of inheritance allowed by the Google style guide, for which we do not require classes to have the “Interface” suffix). Consider whether composition could solve the problem instead.

I will use composition instead to solve this problem.
Cc: rohitrao@chromium.org
[Bulk Edit]

I'm not sure those bugs are worth fixing.

First our observer classes are interface-ish classes (they are not named XXXInterface because it make much more sense to name them XXXObserver) and if they have implementation for their method they are empty (i.e. you never have to call the super class implementation in the derived class). So those inheritance are not cases of implementation inheritance.

Second, those classes (e.g. WebFaviconDriver) are designed to mimic their chrome/ counterpart (e.g. ContentFaviconDriver) and removing the multiple inheritance from the ios/chrome/ implementation would need to be done in sync with removing the inheritance from the chrome/ class too.

Finally some of the bugs are incorrect as the classes inheritance correspond to what is allowed by the style guide (e.g. IOSChromeSyncedTabDelegate inherits from sync_sessions::SyncedTabDelegate and web::WebStateUserData<IOSChromeSyncedTabDelegate> where sync_sessions::SyncedTabDelegate is a pure interface). 
Components: UI>Browser>ReaderMode
Status: WontFix (was: Started)

Sign in to add a comment