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
17
I think this question is unclear because you haven't given us any idea of whatBase
,A
,B
,C
, and_field1
are. Those are important details that should not be left out; I think you should edit the question to talk about what those are.
– Tanner Swett
Jan 4 at 21:12
Based on the answers I would: rebuild Vehicle to have a virtual Suspension, then Car and Bicycle could have Wheels and Boat could have Buoyancy, which will move the abstraction upwards. I find that if my abstraction leads directly to specific settings then I haven't moved the concept far enough up the chain.
– Patrick Hughes
Jan 4 at 21:19
5
Don't use class inheritance to avoid duplicating code. Use it for inheriting and extending behavior, i.e. polymorphism. Move a common field to a base class if and only if it is logically the same field, not two unrelated pieces of information which happen to share the same name in their respective contexts.
– Brandon
Jan 5 at 0:01
Why do you have a base class to begin with?
– jpmc26
Jan 5 at 1:13
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 Jan 4 at 17:02
samis
asked Jan 4 at 16:58
samissamis
203210
203210
17
I think this question is unclear because you haven't given us any idea of whatBase
,A
,B
,C
, and_field1
are. Those are important details that should not be left out; I think you should edit the question to talk about what those are.
– Tanner Swett
Jan 4 at 21:12
Based on the answers I would: rebuild Vehicle to have a virtual Suspension, then Car and Bicycle could have Wheels and Boat could have Buoyancy, which will move the abstraction upwards. I find that if my abstraction leads directly to specific settings then I haven't moved the concept far enough up the chain.
– Patrick Hughes
Jan 4 at 21:19
5
Don't use class inheritance to avoid duplicating code. Use it for inheriting and extending behavior, i.e. polymorphism. Move a common field to a base class if and only if it is logically the same field, not two unrelated pieces of information which happen to share the same name in their respective contexts.
– Brandon
Jan 5 at 0:01
Why do you have a base class to begin with?
– jpmc26
Jan 5 at 1:13
add a comment |
17
I think this question is unclear because you haven't given us any idea of whatBase
,A
,B
,C
, and_field1
are. Those are important details that should not be left out; I think you should edit the question to talk about what those are.
– Tanner Swett
Jan 4 at 21:12
Based on the answers I would: rebuild Vehicle to have a virtual Suspension, then Car and Bicycle could have Wheels and Boat could have Buoyancy, which will move the abstraction upwards. I find that if my abstraction leads directly to specific settings then I haven't moved the concept far enough up the chain.
– Patrick Hughes
Jan 4 at 21:19
5
Don't use class inheritance to avoid duplicating code. Use it for inheriting and extending behavior, i.e. polymorphism. Move a common field to a base class if and only if it is logically the same field, not two unrelated pieces of information which happen to share the same name in their respective contexts.
– Brandon
Jan 5 at 0:01
Why do you have a base class to begin with?
– jpmc26
Jan 5 at 1:13
17
17
I think this question is unclear because you haven't given us any idea of what
Base
, A
, B
, C
, and _field1
are. Those are important details that should not be left out; I think you should edit the question to talk about what those are.– Tanner Swett
Jan 4 at 21:12
I think this question is unclear because you haven't given us any idea of what
Base
, A
, B
, C
, and _field1
are. Those are important details that should not be left out; I think you should edit the question to talk about what those are.– Tanner Swett
Jan 4 at 21:12
Based on the answers I would: rebuild Vehicle to have a virtual Suspension, then Car and Bicycle could have Wheels and Boat could have Buoyancy, which will move the abstraction upwards. I find that if my abstraction leads directly to specific settings then I haven't moved the concept far enough up the chain.
– Patrick Hughes
Jan 4 at 21:19
Based on the answers I would: rebuild Vehicle to have a virtual Suspension, then Car and Bicycle could have Wheels and Boat could have Buoyancy, which will move the abstraction upwards. I find that if my abstraction leads directly to specific settings then I haven't moved the concept far enough up the chain.
– Patrick Hughes
Jan 4 at 21:19
5
5
Don't use class inheritance to avoid duplicating code. Use it for inheriting and extending behavior, i.e. polymorphism. Move a common field to a base class if and only if it is logically the same field, not two unrelated pieces of information which happen to share the same name in their respective contexts.
– Brandon
Jan 5 at 0:01
Don't use class inheritance to avoid duplicating code. Use it for inheriting and extending behavior, i.e. polymorphism. Move a common field to a base class if and only if it is logically the same field, not two unrelated pieces of information which happen to share the same name in their respective contexts.
– Brandon
Jan 5 at 0:01
Why do you have a base class to begin with?
– jpmc26
Jan 5 at 1:13
Why do you have a base class to begin with?
– jpmc26
Jan 5 at 1:13
add a comment |
4 Answers
4
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.
3
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
Jan 4 at 17:21
11
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
13
@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
Jan 4 at 19:09
10
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
8
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
|
show 5 more comments
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
Jan 4 at 18:34
add a comment |
I'm going to play devil's advocate here.
Right now you should do nothing.
Is it DRY? No. But it's better to have a little duplication than a premature abstraction that you can't easily back out of later. The refactor to move a property to a common base class is easy. Going the other way isn't. Wait and see.
When making this sort of decision I tend to use a "rule of 3": once I have repeated the same thing in e.g. three different places, and only then, do I consider moving it up the chain. N.B. you're only at 2.
1
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
1
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days 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.
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
1
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days ago
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
4 Answers
4
active
oldest
votes
4 Answers
4
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.
3
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
Jan 4 at 17:21
11
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
13
@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
Jan 4 at 19:09
10
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
8
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
|
show 5 more comments
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.
3
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
Jan 4 at 17:21
11
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
13
@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
Jan 4 at 19:09
10
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
8
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
|
show 5 more comments
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 Jan 4 at 17:02
PetePete
2,5801714
2,5801714
3
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
Jan 4 at 17:21
11
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
13
@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
Jan 4 at 19:09
10
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
8
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
|
show 5 more comments
3
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
Jan 4 at 17:21
11
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
13
@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
Jan 4 at 19:09
10
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
8
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
3
3
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
Jan 4 at 17:21
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
Jan 4 at 17:21
11
11
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
A boat has 0 wheels. What does that break?
– D Drmmr
Jan 4 at 18:51
13
13
@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
Jan 4 at 19:09
@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
Jan 4 at 19:09
10
10
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
My point is that the example is bad. There is nothing conceptually wrong with a vehicle (that happens to be a boat) stating that it has 0 wheels.
– D Drmmr
Jan 4 at 19:50
8
8
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
Then it all goes to hell when you need to store a paddle boat.
– IllusiveBrian
Jan 5 at 2:04
|
show 5 more comments
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
Jan 4 at 18:34
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
Jan 4 at 18:34
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 Jan 4 at 17:21
answered Jan 4 at 17:16
Erik EidtErik Eidt
22.6k43158
22.6k43158
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
Jan 4 at 18:34
add a comment |
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
Jan 4 at 18:34
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
Jan 4 at 18:34
This is actually what I've decided to do (A and B overlap mostly, so B will derive from A).
– samis
Jan 4 at 18:34
add a comment |
I'm going to play devil's advocate here.
Right now you should do nothing.
Is it DRY? No. But it's better to have a little duplication than a premature abstraction that you can't easily back out of later. The refactor to move a property to a common base class is easy. Going the other way isn't. Wait and see.
When making this sort of decision I tend to use a "rule of 3": once I have repeated the same thing in e.g. three different places, and only then, do I consider moving it up the chain. N.B. you're only at 2.
1
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
1
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days ago
add a comment |
I'm going to play devil's advocate here.
Right now you should do nothing.
Is it DRY? No. But it's better to have a little duplication than a premature abstraction that you can't easily back out of later. The refactor to move a property to a common base class is easy. Going the other way isn't. Wait and see.
When making this sort of decision I tend to use a "rule of 3": once I have repeated the same thing in e.g. three different places, and only then, do I consider moving it up the chain. N.B. you're only at 2.
1
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
1
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days ago
add a comment |
I'm going to play devil's advocate here.
Right now you should do nothing.
Is it DRY? No. But it's better to have a little duplication than a premature abstraction that you can't easily back out of later. The refactor to move a property to a common base class is easy. Going the other way isn't. Wait and see.
When making this sort of decision I tend to use a "rule of 3": once I have repeated the same thing in e.g. three different places, and only then, do I consider moving it up the chain. N.B. you're only at 2.
I'm going to play devil's advocate here.
Right now you should do nothing.
Is it DRY? No. But it's better to have a little duplication than a premature abstraction that you can't easily back out of later. The refactor to move a property to a common base class is easy. Going the other way isn't. Wait and see.
When making this sort of decision I tend to use a "rule of 3": once I have repeated the same thing in e.g. three different places, and only then, do I consider moving it up the chain. N.B. you're only at 2.
answered Jan 4 at 22:02
Jared SmithJared Smith
1,187517
1,187517
1
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
1
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days ago
add a comment |
1
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
1
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days ago
1
1
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
A wise observation that is, I would say, required for making the decision.
– radarbob
Jan 4 at 22:25
1
1
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days ago
I mostly agree, but "Right now" (with no extra code as the one we see) the refactoring in both directions is trivial. But if there is additional code which uses the field, refactoring in both directions can become significantly harder.
– Doc Brown
2 days 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.
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
1
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days 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.
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
1
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days 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.
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 Jan 4 at 19:16
answered Jan 4 at 19:05
GrandmasterBGrandmasterB
35.1k569122
35.1k569122
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
1
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days ago
add a comment |
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
1
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days ago
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
My example is trivial, though it is still production code. You make a good argument for "compromising" the hierarchy with a "heavy" base class, which I too would see myself leaning towards in such a case.
– samis
Jan 4 at 20:11
1
1
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days ago
@samis: you use classes named "A, B, C" and "Base" with just one field and no method in production code? I question that.
– Doc Brown
2 days ago
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
17
I think this question is unclear because you haven't given us any idea of what
Base
,A
,B
,C
, and_field1
are. Those are important details that should not be left out; I think you should edit the question to talk about what those are.– Tanner Swett
Jan 4 at 21:12
Based on the answers I would: rebuild Vehicle to have a virtual Suspension, then Car and Bicycle could have Wheels and Boat could have Buoyancy, which will move the abstraction upwards. I find that if my abstraction leads directly to specific settings then I haven't moved the concept far enough up the chain.
– Patrick Hughes
Jan 4 at 21:19
5
Don't use class inheritance to avoid duplicating code. Use it for inheriting and extending behavior, i.e. polymorphism. Move a common field to a base class if and only if it is logically the same field, not two unrelated pieces of information which happen to share the same name in their respective contexts.
– Brandon
Jan 5 at 0:01
Why do you have a base class to begin with?
– jpmc26
Jan 5 at 1:13