|
The real problem with your proposition here is that you are confusing multiple concepts, and trying to imply they are equal to each other.
First, as has been said in many of the comments. Attributes are metadata, not code. Even if they contain code, that code is separate classes attached to the Metadata, which is itself linked to the class.
Adding or removing attributes (or any metadata) does not alter the class in any way. It does alter the package the class ships in, and it does alter the source file, but these are different from altering the class itself.
This is like saying that a C# extension method violates the SRP when in fact it does not, and in fact is a great example of honoring the OCP. Open for extension but closed for change. In my mind, attributes work exactly the same as Extension methods. In effect, both extension methods and Metadata are *decorators*. And decorators are the very antithesis of violating SRP because they add additional responsibilities in separate classes.
Your examples are problematic for a number of reasons. You base "change" on whether you have to edit the source file. By that definition, two classes in the same source file means that changing one class violates SRP. That's simply not the case. SRP applies only to the *definition* of the class itself, and only the class. In particular, SRP refers to the *contract* of the class (both behavior and structure).
Adding an attribute does not change the behavior or the structure of the class. It may change the behavior of code which uses the class, but not the class itself. This would be like claiming that an orangutan is a chimpanzee because you put him in the cage labeled "chimpanzee" instead of the one labeled orangutan. The fact that these "labels" are compile-time rather than run-time is simply an implantation detail of the compiler, which could be changed at any time without the class having any knowledge of that change.
And that's the key part... If a new version of the .NET runtime made attributes dynamic and runtime (and i'm not sure if this might even be possible with Roslyn), the class would not have to change at all to add the functionality of serialization or Display names... The class would be unchanged. I'll say it again.. It's merely an artifact of the compiler implementation that the class has to be recompiled, not the class itself.
You basically have it backwards. Attributes are the opposite of an SRP violation, and they are the epitome of an Open Closed Principle implementation.
--
Where are we going? And why am I in this handbasket?
|
|
|
|
|
You say that I got things backward, but apparently you didn't understand what I said.
For example, some people understand the open-closed principle like you. As long as you don't change the code of a class, nothing is violated, then attributes are seen as metadata (again, as you said). I will not discuss if the principle implies that or not as many people understand it that way. Yet, to me it is stupid to interpret closed as "you can edit the file when you want, as long as you don't change the code inside the class". It defeats the purpose. To me a good architecture is one that supports extension to closed libraries. In that case, you can't change the source code at all. It's not important if everything was written in a single unit or in 300 different units.
So, taking that into account, frameworks based on attributes may force developers to violate that principle as I interpret it. If it is not what the OCP states, I don't care. Call it the Paulo Zemek's Open-Closed Library principle. But I was talking about Single Responsibility Principle, not OCP. So, the Single Responsibility Principle says that a class should have only one reason to change. And you defend that the class is not changing by putting attributes. Well, the problem is: You can't "attach" attributes to a class when you don't have its source code. I don't care if in your vision its not code. The problem is that you must have the source code of that class and change it. Except from attributes used over classes (that you can still insist that are "outside" the class and I will not try to convince you on the contrary), attributes used over methods, properties etc are written inside the class. Saying that it is attached and that it is metadata doesn't change anything. I can say that all methods aren't part of the class. They are simply attached to a class (and guess what, that's what happens with extension methods... and to me extension methods have nothing to do with OCP, as you can't create virtual extension methods).
And I must ask: Why is this code:
VerifyNotNull(() => Property);
VerifyRange(() => Property, 0, 100);
While this is not:
[VerifyNotNull]
[VerifyRange(0, 100)]
Call it metadata or whatever you want. In both cases we have 2 lines of code.
The important thing is: Where do you do the change?
If you have to put an attribute to an existing class you need its source code. If you need to implement a virtual method or add an interface you need to change its source code. If you can create another class called "ObjectValidator" that works as the validator for the original class, then it is OK. If it uses attributes or methods will not be important. The problem is that attributes, exactly because they are seen as metadata and not code, create bad frameworks and the developers then need to make their classes considering all the possible frameworks in which those classes will be used. It shouldn't be like that. A class made to contain user information should have only that. Consider it will be put into the Data-Access Layer library. Information about serialization, descriptions, help, which fields must be shown in red in the screen etc can exist on their own units, which will probably be in their own libraries too.
About attributes being compile-time and not run-time, I agree that it would be possible to change .NET to allow attributes to be added at run-time. As it would be possible to add new virtual methods to classes at run-time. Yet, it would be a fundamental change to .NET. It has nothing to do with the compiler causing the limitation, as .NET itself doesn't allow that. Roslyn can't change that as it compiles .NET IL. And actually allowing attributes to be added at run-time at this moment will impose security threats.
And about your chimpanzee/orangutan example, the problem is not that chimpanzee is written in the cage's label. The problem is that for the "framework" workers to identify if it is a chimpanzee or orangutan they don't look at the animal as a whole, they look at the tattoo on its forehead. And some workers look for a red tattoo while others look for a blue tattoo, so all animals must have two tattoos (similar to being forced to put [Serializable] and [XmlSerializable] to classes). If tomorrow third party employees from another company are hired to help with teach them hand signals, all of them will need new tattoos because the employees only accept to work with animals that have their specific tattoos.
modified 8-Feb-15 6:26am.
|
|
|
|
|
You don't actually need the source code to attach attributes. You can use any number of techniques to do so, such as injecting IL into the executable... You can even "copy" the class in memory and emit a new identical class with the attributes (or without) using basic reflection and codedom. However, I see that as beside the point...
You're right, dynamic languages allow you to attach methods at runtime. Languages like Smalltalk have been doing this for decades. C# can do this with dynamic and expando objects. So I really don't get your comment that this is somehow a security issue, since it makes no sense that you can completely alter dynamic objects at runtime, but somehow adding or removing attributes from them would be a security violation? I don't buy it.
The reality is that dynamic and runtime code generation more or less falls outside the realm of SOLID design, which is rooted in static design principles. It wasn't written to take these concepts into account, nor was it written to take metadata into account. So you can argue that Emit or dynamic violate SRP as well, but that's because they simply don't consider these aspects of design, which means that it simply doesn't apply to them. Much like trying to apply certain laws of physics to quantum mechanics, which in certain cases do not apply or do not apply the same way.
No matter how you want to spin it. Attributes do not change the responsibilities of the class. Adding [Serializable] does not add additional code to the class, nor does the class perform any additional work.
SRP was never intended to refer to recompiling as "change", since we recompile code all the time even if there isn't any change (ie Rebuild Solution). This argument is silly in so many ways because there are WAY too many things that can require a recompile. For instance, changing a dependent type in a way that does not break LSP (ie, let's say I updated a comment in the source file of a type passed as a parameter. The code file gets recompiled, causing our class to be recompiled).
Your argument basically revolves around a twisted definition of "change". Metadata, by its definition, is external to the object or it wouldn't be metadata. Altering a files read-only attribute on a filesystem does not alter the file, but it certainly changes what you can do with it as an external actor. Attributes are absolutely no different.
--
Where are we going? And why am I in this handbasket?
|
|
|
|
|
"You don't actually need the source code to attach attributes. You can use any number of techniques to do so, such as injecting IL into the executable... You can even "copy" the class in memory and emit a new identical class with the attributes (or without) using basic reflection and codedom. However, I see that as beside the point... "
How many people and how many frameworks are really capable of doing this?
Note that under constrained environments you can't emit code. CodeDOM is much less powerful than you think. If you don't have the source code already, I doubt you will be able to do anything like what you said.
"...C# can do this with dynamic and expando objects. So I really don't get your comment that this is somehow a security issue, since it makes no sense that you can completely alter dynamic objects at runtime, but somehow adding or removing attributes from them would be a security violation? I don't buy it."
Changing objects that were created since the start as dynamic is not a problem, as the user of such objects expects them to be changing. Changing a 3rd party component that used attributes as some kind of guarantee to talk to another framework and giving the changed objects back to that framework is a security issue.
In this case, we can see the same issue when changing immutable objects.
"The reality is that dynamic and runtime code generation more or less falls outside the realm of SOLID design, which is rooted in static design principles. It wasn't written to take these concepts into account, nor was it written to take metadata into account..."
Maybe you are right. But what I am saying is that those principles were created to make code easier to maintain and to avoid having to go back to old code every time a new feature is added. And frameworks that require attributes end-up forcing developers to do that.
Also, you seem to forget that in many situations we are already handed fully created instances. It doesn't matter if we can dynamically create a new library that looks the same at run-time and uses the new attributes. It is far from a common practice, an easy one and definitely it will not help when you are already receiving fully created instances of types that don't have a stupid attribute that could be very well attached at run-time by using a dictionary.
"So you can argue that Emit or dynamic violate SRP as well, but that's because they simply don't consider these aspects of design, which means that it simply doesn't apply to them. Much like trying to apply certain laws of physics to quantum mechanics, which in certain cases do not apply or do not apply the same way."
On the contrary. I love to emit code at run-time as a way to avoid writing repetitive code. And I write my frameworks to take advantage of those features when it is valid to do so. The users of my framework will only benefit from this.
Now saying that my frameworks will use attributes and everybody will need to change all their classes, be it manually or using any of the "techniques" you are describing completely violates the spirit of those principles. So, it doesn't matter if those principles didn't take [Attributes] into account. See the spirit and you will see that if you need to enter half the units of your application to put an attribute only to be able to use the new library, then something is wrong. That is, in many cases a registration "by convention" makes much more sense and can be applied to existing classes without having to enter their source code, being really "attached" to the classes.
I can agree that if attributes could be easily attached at run-time my discussion will be really wrong. But I really hope this doesn't change now because, as I said, it will be a security issue (ah... even the .NET security is based on attributes, I hope nobody is able to remove these attributes at will).
"No matter how you want to spin it. Attributes do not change the responsibilities of the class. Adding [Serializable] does not add additional code to the class, nor does the class perform any additional work."
Adding [Serializable] to the class is the same as creating a static constructor and putting:
Serializer.MakeSerializable(typeof(ActualClassName));
Then, putting the XmlSerializable attribute is the same as:
XmlSerializer.MakeSerializable(typeof(ActualClassName));
And putting a [Description] over a property is the same as:
Description.Set(() => PropertyHere, "Description here");
The difference is that if it is made by methods we can call that from outside the class, by a completely different class and even by a different library. Making it by attributes we can't.
Again, if we are talking about security traits, it is desirable that only the class itself is able to do it (yet I can see it happening from other classes as long as they are in the same assembly).
"SRP was never intended to refer to recompiling as "change", since we recompile code all the time even if there isn't any change (ie Rebuild Solution). This argument is silly in so many ways because there are WAY too many things that can require a recompile. For instance, changing a dependent type in a way that does not break LSP (ie, let's say I updated a comment in the source file of a type passed as a parameter. The code file gets recompiled, causing our class to be recompiled)."
You still completely miss the point of 2 or more libraries. Imagine that I will provide you (or someone else, it doesn't matter) with the data-structures that need to be filled to call a service, which I am also providing in a closed DLL. If I decide to change the DLL, I am free to do it as long as I don't break the existing API. I am free to add attributes, comments and to recompile how many times I want.
The users of my library will not be able to do it, even if it is to put a simple attribute. If someone asks me to put [Serializable] in all my data-structures because he is using those structures directly in situations I wasn't expecting, well, I will probably ask him to use a different serializer, because I will not change my classes for that reason. If someone else asks me to put [XmlSerializable] because they want to save XML documents with my classes... great, I will probably be using XML already when doing the service calls, but I will not change my data-structures for that. If someone wants to create automatic forms with the classes and want a better label than the property names. Great, they can do it, as long as they attach the information they need. I will not do it. Even if it is easy, it is not my responsibility. It is not the responsibility of my classes.
Users will always have the possibility of work-arounds (creating new classes that add the attributes and redirect to mine) but as a work-around, they will need more code for no reason. If those frameworks weren't solely based on attributes, they will not require that.
So, I ignore your interpretation of change to a class or to metadata of the class. I talk about libraries. If you are having to change a base library by the needs of a dependent library, something is wrong. And attributes are usually in this category. I don't say always, but it is a common mistake.
And talking about files, do you use code versioning tools? I prefer to see the history of a file and see all the times that it was changed because a real need (like a bug fix) and not to see that:
* When project A decided to serialize those objects, we entered all the files in this project to put serializable.
* When project B decided to improve on project A, they used XmlSerialization and we needed to change all the files in this project to include the new attribute.
* Developers of project C created their own attributes, and as they use our objects directly and want to avoid creating unnecessary copies, we came to half of our classes to put the attribute needed by C.
So, if you still don't see a problem with this, then there's nothing to discuss.
|
|
|
|
|
I've already said my peace on most of this, but one thing:
Paulo Zemek wrote: Adding [Serializable] to the class is the same as creating a static constructor and ...
A static constructor is executed when an instance of the class is created. Attribute classes are not statically created when the class is instantiated. Only when the attribute is Accessed (ie via GetCustomAttribute()).
So no, these are not even remotely similar.
--
Where are we going? And why am I in this handbasket?
|
|
|
|
|
Yes... you "are right". "It doesn't matter that the end-result can be exactly the same, what matters is the time when things are executed."
Honestly, you aren't supposed to know when attributes are loaded and attached to types. When you request them, you get a copy. If registering them is actually executed as part of the library loading, together with the static constructor or etc is an implementation detail, and the MONO implementation can be different from the Microsoft one.
You are really right that we can always change closed sourced libraries by emitting new code and injecting things. And we can also always access a private member by using unsafe code and unsafe reflection. So, any principle is moot. We can always make unmaintainable code maintainable by adding more code, adapters, façades and layers.
So, this discussion is moot too. If you want, feel free to vote one. I know that I will not change your opinion, and you will not change mine. The most incredible is that I am able to use the Single Responsibility Principle and the Open-Closed Principle with my strict view of what's a change.
That is, maybe you are right by your interpretation of the principle. Maybe the original idea of the principle really accepts that SRP allows you to keep modifying the file as long as you don't modify the class, and attributes are "attached". So, I am really talking about my interpretation of SRP. And I will probably write an article about my view on SOLID. To me, SOLID doesn't state anything about attributes, but it states ideas. By my view and keeping the ideas in mind, changing a unit by any reason that's not its main purpose is a violation of the single responsibility principle... for that UNIT.
That is, a unit should contain a single reason to change, and this reason is a change to its single class. If you need to change to unit to attach something to the class, there's a violation. If you put 2 classes in the unit, there's a violation.
Partial classes solve the problem for the unit (not for the class), by allowing developers to stop changing a specific unit, even to add new methods. The new unit, may or may not be violating the principle according to its purpose. A unit created do add attributes to existing classes will not be violating anything. But by my view the class will by requiring to be recompiled. I don't care if the compiler recompiles the entire project when I do a build. What I care is that I should not need to explicitly ask to rebuild a unit if the SRP is working fine. And this must work in a single project that includes all units or in a "library per class" basis.
In any case, I see this discussion is not going to end. So, keep your idea of single responsibility principle and understand this as "Paulo Zemek's closed view on SOLID principles: Discussion about SRP". I am really thinking about posting an article about my view on SOLID principles. Really, as to me most principles seem to be interpreted in 10 different ways and the discussion is always about a small thing like "the principle doesn't state attributes, libraries, dynamic languages or anything different."
modified 8-Feb-15 20:21pm.
|
|
|
|
|
Hi Paulo,
Good article. I see your point. I think you should've mentioned that, according to SRP, "a class should have only one reason to change" (or something like that).
Let's say you have a class Person which stores Name and DateOfBirth. Now the only reason for this class to change is if we want to store more personal information, like Gender.
Now another reason to change would be to make it Serializable, that seems like a problem...
I've had to 'copy' complete classes from third party libraries because they weren't serializable. The classes did what they had to do, but I couldn't serialize them and to make them serializable I had to change their source (which I couldn't since it was third party). You could even say it violates the open/closed principle.
However, having to create and register a serializer for every type is troublesome as well... That would mean some types could be serializable in one application, but not in another.
Or types could be serialized in multiple ways (because you could create multiple serializers). So how would you know how to deserialize it? Well, allright, we do have multiple serializers now as well...
How would you handle versioning? Let's say I added Gender to the Person class, but I've already serialized it without the Gender attribute. I believe we have the OptionalFieldAttribute for that now.
So while I do see your problem and I agree with it I wouldn't want your solution instead because it creates a lot of work and could be ambiguous and fault tolerant.
It's an OO world.
public class SanderRossel : Lazy<Person>
{
public void DoWork()
{
throw new NotSupportedException();
}
}
|
|
|
|
|
You are right that the same type may be serializable in one application and not in another one.
But if you do things consistently, you will probably have a library for the serializers. So, if you want serialization, you use that library and everything will work (and those serializers can be generic too, so you can have a SerializeAllFieldsFrom<T>).
Also, the fact that some fields may be optional or not can still exist, but they will be put in the item serializer (another item really attached to the primary class) not in the class itself.
|
|
|
|
|
Paulo Zemek wrote: But if you do things consistently If there's one thing I learned about programming it's that nothing is ever consistent!
Generic serializers could work, having some more specific serializers for some types could work... But I'll stick to Microsoft's Serialization for now
I rated this article a 5 by the way. Your points are valid and your solution looks good (though it requires some work).
It's an OO world.
public class SanderRossel : Lazy<Person>
{
public void DoWork()
{
throw new NotSupportedException();
}
}
|
|
|
|
|
The solution in the sample application is far from the final one, that's why I say I don't expect people to use it. It is more of a "proof of concept". Yet I may present the final solution in the future (it already exists, but don't have an article), which is fully configurable and can serialize most [Serializable] classes. The advantage is that it can also serialize the classes not marked as [Serializable].
|
|
|
|
|
Adding attributes to class violates SRP. Data class is responsible of containing data. It is not responsible of telling that I am serializable, I have these validation rules, I have these members with these English field names and I can be used in WCF, this field should not be serialized in JSON, etc. Data class should contain only data. All other things should be placed in other classes. Well, it's easier to say than do in practice because frameworks require you to do things in their way. Sadly.
Microsoft has done many mistakes in .NET design. Many places violates modern principles.
We have had to add [Serializable] into classes because some of them are added to HttpSession (stored in SQL Server). We need to tell that yes, we have thought that this is serializable, and let the default serializer do the job. This is just adding an attribute to class. Why it cannot work without an attribute? If this class is in external library (we have no source code to edit it), we are in trouble.
I am dealing with multilingual systems all the time therefore attribute based techniques with literal text (in English) cannot not be used. Anyway it violates the SRP.
Validation using Data Annotations require attributes. Now you add those attributes to every class. SRP violation. Then you realize that you need some rules that cannot be done with attributes. You decide to do it properly and replace your Data Annotation validation class with FluentValidation and write rules in their own classes. Once you have removed validation rules from data classes validation rules do not violate SRP.
WCF require DataContract and DataMember attributes. If you add those to your domain classes, you violate SRP. You need to violate this, but you should not touch domain classes. You should create separate classes for WCF and add attributes to them. That is keeping internal data structure and external data structure separate.
In practice we have to violate SRP sometimes. It's not our fault.
|
|
|
|
|
Thanks for your comment.
Yes, sometimes we need to violate SRP because we are using those technologies that force us to do so.
But I really hope people start to understand it to avoid such mistakes when creating those technologies (or even their own small and personal frameworks).
|
|
|
|
|
I am not going to post a whole book here. But your missing one main point that derails your entire argument.
Attributes ARE separate classes!
AND
Attributes DO NOT change or affect the class in any way (with the obvious possible exception of Aspect Oriented Programming)
If you notice, class attributes are OUTSIDE a class, they are attached to a class, not included as part of it. Following this logic, any attributes on a class are outside the scope of the class itself, therefore making it impossible for them to violate the SRP of the class.
-Kat
|
|
|
|
|
Attributes are separate classes. True.
Attributes do not change or affect the class in any way. False.
Attributes are outside a class, attached to it. False. Or maybe.
Creating an object of a class really does not need to read the attributes, but they aren't added at run-time. If you use a framework that requires an attribute (I used the [Serializable] as the example) you can't add such attribute to a type without changing the source code, so it is not "attached" considering the run-time. And, if the attribute is mandatory for something to work, it is violating SRP.
But aparently you didn't read the article. You saw the title and downvoted.
|
|
|
|
|
Attributes do not change or affect the class in any way. False. True
attributes can use the class that it is attached to, accessing public properties and so forth, but do not rewrite the class or effect the internal functions of the class.
Attributes are outside a class, attached to it. False. Or maybe. True
So lets take your [Serializable] example... Not having this attribute, does not in any way prevent the class from functioning as written. Anywhere you create the class you can use it and access its public members. Not having the SerializableAttribute on the class prevents A DIFFERENT class from functioning. Wait, doesn't that mean the serialization logic is in another class? Yes it does. And this other class is looking for this EXTERNAL tag to the class to find out if it can handle it? Yes.
This is no way violates the SRP.
Thank you,
Kat
|
|
|
|
|
You are right. Another class is not working. But the code of your class will need to change to include the attribute.
You may argue that it is not executable code. You are right. It is not. But it is code, written by a programmer and it should be in the same unit. It should be "part" of the class, even if you consider it to be attached to the class.
Look at the situation:
public sealed class MyClass
{
public string Name { get; set; }
}
Now, we attach a trait:
AttachTrait(typeof(MyClass), SerializableFlag.Instance);
In this case, MyClass does not need to change.
Now look at the [Serializable] attribute.
Can I say that MyClass is serializable without changing its source code? I am not talking about alternative serializers, so the only way is to go to the unit of that class and change its source code to be:
[Serializable]
public sealed class MyClass
{
public string Name { get; set; }
}
So we needed to change our code for a reason that is not related to the class itself. The attribute is another class? Yes. But the existence of such class is not enough, we need to "attach" that class to our class, but the only way to do it is at compile-time, not at run-time, so we need to change our code. The fact that it is not instruction code is irrelevant as we still need to change the source code.
In fact, I see that the problem here is related to the "attached" problem. To you, attributes are attached to the type, not part of the type, so there is no problem. But to be really attached there should be the support to add them later (and in this case putting them in the main unit will be only a shorthand), but that's not the case. Attributes, like any members, are part of the type description. They don't affect the instances directly, that's true, but any new static method that is never called will not affect any existing instance.
It is another class that will not work if you don't put the attribute, that's also true, but the SRP states that there need to be a single reason to change, and putting the attribute (or attaching the attribute) is a change and, as we can't add it at run-time or in another unit, we should change the source code of that unit.
Imagine that the "MyClass" is already compiled. You need to use it, but you don't have the source. Now you need to "attach" that attribute. Can you?
And that's why I said that attributes should be used as hints. I still use attributes a lot, but they are only one of the ways of telling something. I always let the same thing be told differently. That is, I allow things like Serializer.ConsiderSerializable(typeof(MyClass)) or Description.Set(typeof(MyClass), "This is a test class");
I will naturally put the [Description] attribute in my classes, but if I get a third-party type, it will work, because I don't make them the only way. If they are the only way, then you will need to change the source code of a type to "attach" things.
|
|
|
|
|
You kind of make my argument for me. If you do not know the source of the target class, i.e. someone else wrote it. How can you be sure that it is inherently serializable. That is why the attribute usage here makes since.
The Default Serializer has basically said, "Hey, class author, if your class is inherently serializable, let me know by attaching an instance of this class to your type.
If your not the class author, you cannot know that, and therefore should use other methods to serialize the object if absolutely necessary.
Even if the underlying class is inherently serializable, you should not assume that it will always be the case as the class author is obviously not assuming that as well. He/She could update the class later with something that the default serializer cannot handle and then your application just goes BOOM, whereas if you had used another method to serialize it yourself, you could apply the update most likely without issue.
All of this points specifically that an attribute on the class has absolutely nothing to do with the class' internal and proper function itself, and therefore has nothing to do with whether the class conforms to the SPR or not.
-Kat
|
|
|
|
|
The Serializable attribute is only one example (the description is another one much less problematic). But even in the serializable case, if you are the author of the class and you need to change the code of the class to include an attribute because there is no other way of doing it, then the attribute is violating SRP.
If you don't agree, OK. To you attributes aren't code. To me, they are. If I need to open a .cs to change anything in it, I consider I am changing code.
But you say that attributes have nothing to do with the class itself. And that's exactly why they violate SRP. They are written in the class, but they have nothing to do with the main purpose of the class. Or better, they usually don't have.
If I create a base class that expects some attribute on the child classes, it will be OK to put the attribute as it will be part of the responsibility of the inheriting classes. But if I am putting attributes in my classes to allow other code to work, then I am adding a responsibility.
The serializable attribute is a small responsibility (the author of the class must remember about it, or later he will have to change his code to add it). The entire serialization process is broken, IMO, because we can't say that a 3rd party class is serializable and we can't add a serializer either. But the focus is not on serialization. Is on attributes and code changes.
Any framework that expects an attribute on your code, to do something else with it, is forcing you to change existing classes, even if the change is small. This is a violation of SRP (you are changing code for another reason) and also of OCP (you are changing existing types instead of adding that in new types).
So, to me:
serializer.RegisterSerializer(typeof(MyClass), new MyClassSerializer());
Is OK. I don't know if MyClass has private data that should not be serialized, but I know how to recreate new instances without problems, so I add a new serializer. The Color type is an example. I know it has ARGB traits. How they are stored internally is not important, I can read the ARGB values write them to a byte stream and later I can read them and create a new Color instance. So, problem solved.
Description.Set(typeof(MyClass), "This is a sample class with a description");
It is OK too. The MyClass don't need such information.
But changing the MyClass source code to have this:
[Serializable]
[Description("The designer will not show any help if I don't put the description here"]
Means there is a change on existing code. OCP is violated (as my code was open for modifications). SRP is violated because such code means: I am the responsible of telling that I am serializable and also I am the responsible for giving the right description!
|
|
|
|
|
Herein lies the flaw, a cs file is not a class. (Now I do keep single classes in individual cs files myself, except for rare cases)
Adding an attribute to a class is NOT editing the class its editing the cs file Hell, you could add the attribute in a separate file with a partial class declaration if you REALLY want to, but I wont be doing that.
I'm going to say we must agree to disagree here.
|
|
|
|
|
You can add attributes in another file with partial classes. True.
But the attribute is still part of the class.
We can really discuss where is the limit. In fact, any non-virtual instance method that lives in a class is the same as an static method that lives in the class and receives as first parameter the instance to work on. It will be exactly the same IL code to do the call. Yet we consider that static methods are different than instance methods (and the fact that in C# we call them differently enforces that).
To me that's the same problem with attributes. They are not executable code, so you see them as different. But the problem is that you need to modify the definition of the class. Be it a partial class or not, you modify the class itself to have the attribute. In fact, partial classes are a compiler view. The generated classes aren't split into two classes with the same name. Only a single class is generated.
And that's where all the problem lives. If you need to recompile your type so it can be used by something else, there is a violation on SRP. If you agree that attributes are code or not is not the problem. You need to recompile your type to add that attribute. If you get a compiled library, you can't simply add attributes to those types if the attributes are valid for your application.
|
|
|
|
|
Maybe I should expose the problem differently... more related to OCP than to SRP.
If I have a tested library that works, I will close the entire library for modifications. Modifications will only be done to correct bugs. Nothing else. If the modification is in a class, attribute or whatever, it is in the library, and the entire library is closed.
So, all the components work fine in their main purpose. But then someone wants to use that component with another library. Serialization is an example. All the types are serializable by their structure (they only use primitive types and strings, for example). Yet, the .NET BinaryFormatter will not be able to serialize it. My solution? I have a serialization framework that allows serializers to be added, but that's not the point either. Many people, in that particular situation, will reconsider opening the code to put [Serializable] in all their types. The reasons will usually be: It is a small modification... it is much better than having to do all the serialization by hand (surely, it is, but I never said serialization is bad, I only consider the .NET serialization flawed).
Then imagine other situations. The main application of the company has an "edit workflow" feature, which uses the [Description] attributes of the objects to show a good description. But, hey, your library was created without the idea of being used in such workflow, but your components are compatible with the workflow framework. There is only one thing missing. The descriptions. But again, instead of using a solution that's capable of searching for descriptions elsewhere (and maybe using the attribute as one of the ways) the framework requires the attributes. So you should open your library again and put all the descriptions in the code.
But, hey, people loved attributes, and they are using them for everything. They now are marking objects that can be stored in the database with [POCO] attributes. And again, all your types are OK with the structure, but they don't have such attribute. What you do? You change all the types again to put that [POCO] attribute.
Is that a bad practice? Surely, but the [Serializable] is an example of such bad practice.
So, what really happened?
You modified your code many times because of external factors. If those external factors could be really external (live inside another DLL, be in a text file or whatever) it would be much better.
Is the SRP violated? If you insist that attributes aren't part of the class, then it is not. But you should agree that OCP was violated.
But now look at how IL works. Look at how attributes are get. You don't get all attributes from the assembly and then use a property to know where they were applied. You must enter the type (or any of its members) to look for the attributes. That's because they are part of that type.
In fact, even a constant is part of a type. Surely that when compiling code that uses a constant the compiler "copies" the value directly, but the constant continue to exist on the type, which the sole purpose of letting future code to reference it. If you add a new constant to a type, I consider that you changed that type, even if all the references to that constant are replaced by its actual value.
|
|
|
|
|
Also, to compare what is code and what is not. Is adding a new private constructor changing the class?
I am pretty sure you don't consider constructors as items "attached" to a type.
So, let's use a constructor as example. Imagine that instead of doing:
[Serializable]
public sealed class MyClass
{
public string Name { get; set; }
}
You did:
public sealed class MyClass
{
public MyClass(){}
private MyClass(Serializable ignoredParameter){}
public string Name { get; set; }
}
Let's consider that attributes don't exist, but reflection do. So, instead of searching for an attribute, the framework will search for a private constructor that receives a parameter of type Serializable. But such Serializable type is a class that can never be instantiated.
What I did with that was mark the class with an "I am serializable", but using a constructor that will never be invoked instead of using an attribute. Is a new constructor that is never invoked changing the behavior of the class? No.
Is the existence of such constructor making the code less maintenable? No. Or, maybe. You before used the example: What if I mark a class as serializable but, in the future, the class changes and has a field that can't be serialized?
Well, that means that by having the [Serializable] attribute, if you decide or need to add a field you should also take into account if the type will continue to be serializable. So, such attribute is introducing a difficulty to maintain the code. Or should I say such constructor, as I used the constructor as such flag?
So, why is a constructor that's never invoked different from an attribute? If I need to put the attribute on the type and recompile it, it is the same as adding a private constructor or property that will never be invoked. It will be there only for reflection purposes, to help some unrelated code, but yet it will be part of my type. And if I am required to have such attribute, private constructor or private property that I will never invoke, then I think that my code has more responsibilities than it should. The code that is executed may continue to work correctly with or without those extra info, but when something else does not work, it is my code that is changing.
|
|
|
|
|
I've read through the thread here and it seems like you haven't written enough software. I'm not talking about dragging and dropping boxes onto a web page, I'm talking slogging through hundreds of classes and their code, with nothing visual going on. I'm also amused by the comments that some of these comments are "academic" and therefore not important. Well, that "academic" stuff was developed to prevent real problems in software development. So, if you think it's "academic" that's because the concept works and you've never encountered the problem it was designed to solve.
You are misunderstanding the single responsibility principle. That is a reason for concern. There is nothing in the principle itself which defines the scope of an object's "responsibility" and gives us clues about where to draw the line, but common sense and experience can provide that.
For example, you're confused why some things are members of the numeric type classes, and other things are shuffled into utility classes like "Math" - common sense is the primary reason, but the other reason is because we can define our single responsibility any way we want. The designers chose to define that responsibility in the case of the Integer class to be "anything having to do with Integers" and they didn't put trig functions in that group because trig functions are not about Integers - but basic addition and subtraction ARE. If I choose to define it that way, I can do that.
You would also do yourself a favor by looking up the history of problems behind serializing objects - it has almost always been understood that the common sense way of doing that is to have objects responsible for their own serialization. You should just be happy you don't have to write your own serializers and unserializers - cuz that was fun, dude, lol
So to boil that down... I think you're missing the point because you lack experience and historical perspective.
|
|
|
|
|
It seems that this message was lost here for some time.
But, I did write lots of software. If my examples maybe gave you the wrong idea that doesn't mean I don't understand SRP, Serialization or the responsibility of base types like int. I was using examples that fit my description, but I may forgot to explain every detail.
So, about the Math situation. In the first .Net version we had the primitive types (some other numeric types) and the Math static class, with methods like Abs, Min, Max etc.
But now there is the BigInteger type, and for such type, methods like Abs, Min, Max etc are inside the type itself. The Math class doesn't have methods to work with the BigInteger and, in fact, the creation of the BigInteger type should not affect the Math type.
So, do we have a problem with the single responsibility here?
Well, we have a kind of problem. The BigInteger works differently from the other types. It was created later, but that's no excuse. If all numeric types considered that it was their own responsibility to have Abs, Min, Max etc, that wouldn't happen. But we could also have many different classes (Int32Math, Int64Math) only to avoid putting all of the methods into the same class.
The problem with where starts and where ends the responsibility here is: What will happen if new types are added?
If we consider that methods like Min and Max aren't the responsibility of the primitive type, then it's OK, but at the same type, why is the responsibility of the Math type to deal with Int32, Int64 and others and not with BigInteger?
This problem is even worse if we consider that Min and Max can simple be generic using IComparable<T>. So, Math having Min and Max will be valid, but as a single generic method. The problem of Math having methods for many types is that, someday, a new type will appear and Math can't deal with it. So, Math has too many responsibilities.
If Min and Max were responsibilities of the numeric types, we will have a better rule... but maybe it will be still too many methods to implement to make a numeric-type work.
So, even if I can agree that simple math is the responsibility of the int type and Abs, Min and Max it not, I can't agree that Math has to deal with many types, but not all. Something is broken. If we had many "Math" types it could look ugly, but it will conform better to the principle... so, there is the right and wrong even when we define a responsibility.
About serialization, I never said that it should not exist a default serialization. But if the default does not work, because you have an external type that is not serializable (and it is inside a big graph), what do you do?
If the framework is smart enough, it could find external serializers. If it is not, you have to implement the ISerializable in a type that contains such item, or you need to create an wrapper object... but you are changing the serialized item itself, not the way the serializer finds the serialization algorithm. But if you don't agree with me, look at Silverlight, which simple does not have a [Serializable] attribute.
|
|
|
|
|
Been so long I forgot what the hell I was talking about but I don't think I meant you.
|
|
|
|
|