When to move a common field into a base class?
I currently have two derived classes, A
and B
, that both have a field in common and I'm trying to determine if it should go up into the base class.
It is never referenced from the base class, and say if at some point down the road another class is derived, C
, that doesn't have a _field1
, then wouldn't the principal of "least privileged" (or something) be violated if it was?
public abstract class Base
{
// Should _field1 be brought up to Base?
//protected int Field1 { get; set; }
}
public class A : Base
{
private int _field1;
}
public class B : Base
{
private int _field1;
}
public class C : Base
{
// Doesn't have/reference _field1
}
object-oriented inheritance abstract-class
add a comment |
I currently have two derived classes, A
and B
, that both have a field in common and I'm trying to determine if it should go up into the base class.
It is never referenced from the base class, and say if at some point down the road another class is derived, C
, that doesn't have a _field1
, then wouldn't the principal of "least privileged" (or something) be violated if it was?
public abstract class Base
{
// Should _field1 be brought up to Base?
//protected int Field1 { get; set; }
}
public class A : Base
{
private int _field1;
}
public class B : Base
{
private int _field1;
}
public class C : Base
{
// Doesn't have/reference _field1
}
object-oriented inheritance abstract-class
add a comment |
I currently have two derived classes, A
and B
, that both have a field in common and I'm trying to determine if it should go up into the base class.
It is never referenced from the base class, and say if at some point down the road another class is derived, C
, that doesn't have a _field1
, then wouldn't the principal of "least privileged" (or something) be violated if it was?
public abstract class Base
{
// Should _field1 be brought up to Base?
//protected int Field1 { get; set; }
}
public class A : Base
{
private int _field1;
}
public class B : Base
{
private int _field1;
}
public class C : Base
{
// Doesn't have/reference _field1
}
object-oriented inheritance abstract-class
I currently have two derived classes, A
and B
, that both have a field in common and I'm trying to determine if it should go up into the base class.
It is never referenced from the base class, and say if at some point down the road another class is derived, C
, that doesn't have a _field1
, then wouldn't the principal of "least privileged" (or something) be violated if it was?
public abstract class Base
{
// Should _field1 be brought up to Base?
//protected int Field1 { get; set; }
}
public class A : Base
{
private int _field1;
}
public class B : Base
{
private int _field1;
}
public class C : Base
{
// Doesn't have/reference _field1
}
object-oriented inheritance abstract-class
object-oriented inheritance abstract-class
edited 2 hours ago
asked 2 hours ago
samis
15218
15218
add a comment |
add a comment |
3 Answers
3
active
oldest
votes
It all depends upon the exact problem you're trying to solve.
Consider a concrete example: your abstract base class is Vehicle
and you currently have the concrete implementations Bicycle
and Car
. You're considering moving numberOfWheels
from Bicycle
and Car
to vehicle. Should you do this? No! Because not all vehicles have wheels. You can already tell that if you try to add a Boat
class then it's going to break.
Now, if your abstract base class was WheeledVehicle
then it's logical to have the numberOfWheels
member variable in there.
You need to apply the same logic to your problem, because as you can see, it's not a simple yes or no answer.
1
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add aroll()
method, at which point the subclass idea is looking prescient.
– user949300
2 hours ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
1
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
add a comment |
Logically speaking, beyond placing the field replicated in subclasses vs. in common in the base class, there is a third option: which is to introduce a new subclass into the hierarchy that has the common properties between the two. @Pete hints at this without fully going there.
Using @Pete's example, we would introduce a (possibly abstract) subclass for Wheeled Vehicle that descends from the original base class — while the two subclasses descend from it. Thus, the original base class is not polluted with wheels, yet the commonality of wheels is DRY (not repeated among subclasses that have wheels).
This may, of course, be overkill for your purposes, but such is supported by the class hierarchy mechanism.
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
add a comment |
In general, I would move it to the base class. I don't think there's an objective yes/no, because there's a trade-off here - carrying unused fields vs reducing complexity.
I typically prefer 'heavy' base classes that contain anything that might be shared. This makes serializing to files simpler since you don't need descendant serializing methods in every derived class. But if you don't have that or a similar issue, or perhaps you need to do everything you can to reduce memory usage, then only keeping the fields where you need them should be fine.
An 'intermediary' class that introduces the common fields will be fine if you have a very limited number of fields. But be aware that approach can dramatically increase complexity if you have dozens of fields used in different combinations, leading to many intermediary classes each introducing a specific set of fields. That can become a maintenance problem.
add a comment |
Your Answer
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "131"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsoftwareengineering.stackexchange.com%2fquestions%2f384980%2fwhen-to-move-a-common-field-into-a-base-class%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
It all depends upon the exact problem you're trying to solve.
Consider a concrete example: your abstract base class is Vehicle
and you currently have the concrete implementations Bicycle
and Car
. You're considering moving numberOfWheels
from Bicycle
and Car
to vehicle. Should you do this? No! Because not all vehicles have wheels. You can already tell that if you try to add a Boat
class then it's going to break.
Now, if your abstract base class was WheeledVehicle
then it's logical to have the numberOfWheels
member variable in there.
You need to apply the same logic to your problem, because as you can see, it's not a simple yes or no answer.
1
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add aroll()
method, at which point the subclass idea is looking prescient.
– user949300
2 hours ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
1
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
add a comment |
It all depends upon the exact problem you're trying to solve.
Consider a concrete example: your abstract base class is Vehicle
and you currently have the concrete implementations Bicycle
and Car
. You're considering moving numberOfWheels
from Bicycle
and Car
to vehicle. Should you do this? No! Because not all vehicles have wheels. You can already tell that if you try to add a Boat
class then it's going to break.
Now, if your abstract base class was WheeledVehicle
then it's logical to have the numberOfWheels
member variable in there.
You need to apply the same logic to your problem, because as you can see, it's not a simple yes or no answer.
1
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add aroll()
method, at which point the subclass idea is looking prescient.
– user949300
2 hours ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
1
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
add a comment |
It all depends upon the exact problem you're trying to solve.
Consider a concrete example: your abstract base class is Vehicle
and you currently have the concrete implementations Bicycle
and Car
. You're considering moving numberOfWheels
from Bicycle
and Car
to vehicle. Should you do this? No! Because not all vehicles have wheels. You can already tell that if you try to add a Boat
class then it's going to break.
Now, if your abstract base class was WheeledVehicle
then it's logical to have the numberOfWheels
member variable in there.
You need to apply the same logic to your problem, because as you can see, it's not a simple yes or no answer.
It all depends upon the exact problem you're trying to solve.
Consider a concrete example: your abstract base class is Vehicle
and you currently have the concrete implementations Bicycle
and Car
. You're considering moving numberOfWheels
from Bicycle
and Car
to vehicle. Should you do this? No! Because not all vehicles have wheels. You can already tell that if you try to add a Boat
class then it's going to break.
Now, if your abstract base class was WheeledVehicle
then it's logical to have the numberOfWheels
member variable in there.
You need to apply the same logic to your problem, because as you can see, it's not a simple yes or no answer.
answered 2 hours ago
Pete
2,3521513
2,3521513
1
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add aroll()
method, at which point the subclass idea is looking prescient.
– user949300
2 hours ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
1
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
add a comment |
1
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add aroll()
method, at which point the subclass idea is looking prescient.
– user949300
2 hours ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
1
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
1
1
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add a
roll()
method, at which point the subclass idea is looking prescient.– user949300
2 hours ago
One could temporarily accept that 0 is a valid numberOfWheels. However, eventually you might add a
roll()
method, at which point the subclass idea is looking prescient.– user949300
2 hours ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
A boat has 0 wheels. What does that break?
– D Drmmr
45 mins ago
1
1
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
@DDrmmr It's not that a Boat has 0 wheels, it that Wheels don't even exist as a concept for a Boat - hence your models shouldn't allow for it.
– Peter M
27 mins ago
add a comment |
Logically speaking, beyond placing the field replicated in subclasses vs. in common in the base class, there is a third option: which is to introduce a new subclass into the hierarchy that has the common properties between the two. @Pete hints at this without fully going there.
Using @Pete's example, we would introduce a (possibly abstract) subclass for Wheeled Vehicle that descends from the original base class — while the two subclasses descend from it. Thus, the original base class is not polluted with wheels, yet the commonality of wheels is DRY (not repeated among subclasses that have wheels).
This may, of course, be overkill for your purposes, but such is supported by the class hierarchy mechanism.
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
add a comment |
Logically speaking, beyond placing the field replicated in subclasses vs. in common in the base class, there is a third option: which is to introduce a new subclass into the hierarchy that has the common properties between the two. @Pete hints at this without fully going there.
Using @Pete's example, we would introduce a (possibly abstract) subclass for Wheeled Vehicle that descends from the original base class — while the two subclasses descend from it. Thus, the original base class is not polluted with wheels, yet the commonality of wheels is DRY (not repeated among subclasses that have wheels).
This may, of course, be overkill for your purposes, but such is supported by the class hierarchy mechanism.
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
add a comment |
Logically speaking, beyond placing the field replicated in subclasses vs. in common in the base class, there is a third option: which is to introduce a new subclass into the hierarchy that has the common properties between the two. @Pete hints at this without fully going there.
Using @Pete's example, we would introduce a (possibly abstract) subclass for Wheeled Vehicle that descends from the original base class — while the two subclasses descend from it. Thus, the original base class is not polluted with wheels, yet the commonality of wheels is DRY (not repeated among subclasses that have wheels).
This may, of course, be overkill for your purposes, but such is supported by the class hierarchy mechanism.
Logically speaking, beyond placing the field replicated in subclasses vs. in common in the base class, there is a third option: which is to introduce a new subclass into the hierarchy that has the common properties between the two. @Pete hints at this without fully going there.
Using @Pete's example, we would introduce a (possibly abstract) subclass for Wheeled Vehicle that descends from the original base class — while the two subclasses descend from it. Thus, the original base class is not polluted with wheels, yet the commonality of wheels is DRY (not repeated among subclasses that have wheels).
This may, of course, be overkill for your purposes, but such is supported by the class hierarchy mechanism.
edited 2 hours ago
answered 2 hours ago
Erik Eidt
22.5k43157
22.5k43157
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
add a comment |
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
1 hour ago
add a comment |
In general, I would move it to the base class. I don't think there's an objective yes/no, because there's a trade-off here - carrying unused fields vs reducing complexity.
I typically prefer 'heavy' base classes that contain anything that might be shared. This makes serializing to files simpler since you don't need descendant serializing methods in every derived class. But if you don't have that or a similar issue, or perhaps you need to do everything you can to reduce memory usage, then only keeping the fields where you need them should be fine.
An 'intermediary' class that introduces the common fields will be fine if you have a very limited number of fields. But be aware that approach can dramatically increase complexity if you have dozens of fields used in different combinations, leading to many intermediary classes each introducing a specific set of fields. That can become a maintenance problem.
add a comment |
In general, I would move it to the base class. I don't think there's an objective yes/no, because there's a trade-off here - carrying unused fields vs reducing complexity.
I typically prefer 'heavy' base classes that contain anything that might be shared. This makes serializing to files simpler since you don't need descendant serializing methods in every derived class. But if you don't have that or a similar issue, or perhaps you need to do everything you can to reduce memory usage, then only keeping the fields where you need them should be fine.
An 'intermediary' class that introduces the common fields will be fine if you have a very limited number of fields. But be aware that approach can dramatically increase complexity if you have dozens of fields used in different combinations, leading to many intermediary classes each introducing a specific set of fields. That can become a maintenance problem.
add a comment |
In general, I would move it to the base class. I don't think there's an objective yes/no, because there's a trade-off here - carrying unused fields vs reducing complexity.
I typically prefer 'heavy' base classes that contain anything that might be shared. This makes serializing to files simpler since you don't need descendant serializing methods in every derived class. But if you don't have that or a similar issue, or perhaps you need to do everything you can to reduce memory usage, then only keeping the fields where you need them should be fine.
An 'intermediary' class that introduces the common fields will be fine if you have a very limited number of fields. But be aware that approach can dramatically increase complexity if you have dozens of fields used in different combinations, leading to many intermediary classes each introducing a specific set of fields. That can become a maintenance problem.
In general, I would move it to the base class. I don't think there's an objective yes/no, because there's a trade-off here - carrying unused fields vs reducing complexity.
I typically prefer 'heavy' base classes that contain anything that might be shared. This makes serializing to files simpler since you don't need descendant serializing methods in every derived class. But if you don't have that or a similar issue, or perhaps you need to do everything you can to reduce memory usage, then only keeping the fields where you need them should be fine.
An 'intermediary' class that introduces the common fields will be fine if you have a very limited number of fields. But be aware that approach can dramatically increase complexity if you have dozens of fields used in different combinations, leading to many intermediary classes each introducing a specific set of fields. That can become a maintenance problem.
edited 21 mins ago
answered 31 mins ago
GrandmasterB
35.1k569122
35.1k569122
add a comment |
add a comment |
Thanks for contributing an answer to Software Engineering Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsoftwareengineering.stackexchange.com%2fquestions%2f384980%2fwhen-to-move-a-common-field-into-a-base-class%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown