|
Member 4487083 wrote: I used to think I was a very good coder because I could write long and complex methods.
If you're writing long complex methods, you're not breaking down your problems correctly. If I was supporting your code, I'd have to hunt you down and remove your DNA from the face of the earth.
Member 4487083 wrote: How would other people see my code?
The above pretty much sums it up...
|
|
|
|
|
Methods that are only one or two lines long probably means your call stacks are absolutely huge and debugging probably feels like a huge expedition. Spaghetti code gets its name not only because of the length of the noodle, but the number of noodles in the bowl.
What are your criteria for breaking functionality out into its own method? Length of method is rarely the best way to decide. Do you use UML or anything to document your code? Are these methods usually private or public? Do your classes and methods make sense when you describe the functionality in a sentence? For instance, a Dog object has a Tail object property which has a Wag() method. So a dog has a tail than can wag.
|
|
|
|
|
My call stacks aren't huge because the smallest methods are usually very simple and don't call much else. It's probably hard to judge it without seeing it. What really stands out to me though is when methods are 500 lines long, or in some cases more than 1000 lines long.
I don't use UML, to be honest, I've never considered using it. I always saw it as a way to communicate ideas to others and not a form of documentation. You could argue that my code is (often) documented by developer (unit) tests though.
I usually break up the code based on whether it makes sense, and reads like a sentence. It certainly isn't broken up based purely on the number of lines.
I try to follow most of the guidelines in 'Clean Code' by Bob Martin. This is a really good book IMO.
|
|
|
|
|
So your call stacks aren't long and your methods are 1 or 2 lines. Yet your code does the same thing as something with methods 500 to 1000 lines long? Either you have 250+ methods, or you aren't comparing code with equivalent complexity.
|
|
|
|
|
T M Gray wrote: So your call stacks aren't long and your methods are 1 or 2 lines. Yet your code does the same thing as something with methods 500 to 1000 lines long? Either you have 250+ methods, or you aren't comparing code with equivalent complexity.
Well, there's a balance between the two really. A single 1000 line method probably isn't ideal, and 1000 single line methods probably wouldn't be ideal either. Is there a problem with writing 250+ methods though? If you have a complex problem then you may create a number of classes and methods for it. Putting 250+ methods in a single class probably isn't great, but if you have that much code then it can probably be split up into multiple classes.
My call stack would be longer, but I wouldn't normally consider them to be too long. I generally don't see this to be an issue. It can also be useful when an exception is thrown and logged (or unhanlded). Exceptions give you the stack trace, and if an exception is thrown in a 500+ line method then it's likely to be more difficult pin point the problem than if it was a 10 line method.
You are right about the complexity in some ways. However, almost all of the long methods I see can easily be broken down into smaller parts. Even breaking them into methods with 100 lines of code would make the code much more readable.
Although I do write single line methods, a lot of code is more than a couple of lines long, but never 500+ lines.
|
|
|
|
|
If you have 250+ methods then I'd say that you immediately violated Single Responsibility.
|
|
|
|
|
Pete O'Hanlon wrote: If you have 250+ methods then I'd say that you immediately violated Single Responsibility
Agreed. SRP would be violated just as much if the same code was within a single method though.
|
|
|
|
|
Indeed - the point I was trying to make was that the code should be better in other classes.
|
|
|
|
|
Member 4487083 wrote: one or two lines
That's probably too short. How often are they called? From how many places?
When I have a really short method that gets called frequently I reach for the C pre-processor and write a macro.
|
|
|
|
|
PIEBALDconsult wrote: When I have a really short method that gets called frequently I reach for the C pre-processor and write a macro.
In C# it will often get inlined by the JIT anyway so there shouldn't be any performance issues.
|
|
|
|
|
Member 4487083 wrote: will often get inlined
But it seems unpredictable.
|
|
|
|
|
Does the performance difference matter? On the occassions where it doesn't get inlined I suspect that the performance difference won't matter in at least 95% of situations. There are other performance disadvantages of large methods though.
|
|
|
|
|
Like what? (Just askin'.)
|
|
|
|
|
PIEBALDconsult wrote: Like what?
I've read a really good blog about this, but I can't find it now. I'm going by memory.
The first potential (but very minor) benefit happens because of the way the JIT compiler works. If you have the following code
if (x) A(); else B(); then only one of the methods (A or B) with be compiled the first time that it is executed.
With smaller methods there is also a greater chance that CPU registers will be used rather than normal memory.
In addition to this there is the inlining which I have already mentioned. So, in some cases smaller methods can out perform larger methods, and most of the time it's probably hard to tell which will perform best. I always code in a way that is easy to read/maintain because that is what we spend most of our time doing and the performance difference is not worth worrying about 99% of the time.
In addition to this it's also worth remembering how slow other things are. If your code does database queries or uses files then the additional time to call another method really isn't going to make a difference.
|
|
|
|
|
Nope - no question here.
The funniest thing about this particular signature is that by the time you realise it doesn't say anything it's too late to stop reading it.
My latest tip/trick
Visit the Hindi forum here.
|
|
|
|
|
A lot of methods that are only 1 line long can be a little annoying too. Of course if it's a public interface and you want to protect your right to change implementation, then it should still be a method. And there are other good reasons, but just breaking up the code "because you can" isn't a good reason.
You might imagine a large house of 10,000 square feet. If that were just one huge room, the house wouldn't be very usable. If there were appropriate sized rooms, of 100 to 300 square feet each or so, then you have a very usable house. If you have many rooms of only 4 square feet each, then you once again have an unusable house. In other words, break it down to an appropriate level, but no more.
There is a quote supposedly by Pascal that goes something like "I'm sorry this is so long, I didn't have time to make it shorter." Making good methods that are short enough (but not too short) takes time to do correctly.
Yes, reading and maintaining long code (or any code) is a skill. You might want to look at Brownfield Application Development in .NET by Belchem and Baley.
What do you mean when you say your code is "very unique"? All else being equal, being different is not good. All it does is slow people down as they try to read a style they're not used to. So they might find it hard to work with, but if you're doing something good then maybe they should be copying you. My kneejerk response is that you're trying to be too clever. Clever is sometimes good but usually not.
modified on Wednesday, November 17, 2010 9:50 AM
|
|
|
|
|
For a little something I'm experimenting with, I'd like to be able to (binary) read and write various types of object, like I can with C. The biggest need currently is byte , long , and Guid , but I'd like a more general solution.
BinaryReader and -Writer won't do it; they only support a few types (not Guid ) and (apparently) not read/write.
So so far I've been working with a FileStream. At this time, I can do byte (which is built into FileStream), and long and Guid via "unsafe" code.
The problem has been in making generic read and write methods:
T Read<T>()
void Write<T>( T Value )
C# doesn't want to make a pointer to a generic type. I got around that (I think) by using a GCHandle, and maybe that's the best I can do.
Anyway... I have a sneaking suspicion that this has been solved before. Shouldn't others have run into the same need?
Does anyone here have information on such a beast?
(If not, I'll write an article when I'm done.)
|
|
|
|
|
I know you don't like Convert class much, however this seems an opportunity for using BitConverter[^] rather than unsafe code.
|
|
|
|
|
Luc Pattyn wrote: I know you don't like Convert class much
Is it that obvious?
I don't see Guid, or DateTime, or other types I may want in the future.
Guid has a ToByteArray method I tried out.
But I also need to convert back from byte array to the type as well.
|
|
|
|
|
|
|
I remember some years ago I tried something similar just for fun. I used reflection recursively to reduce the object to its minimal parts and then writting its binary data was far easy.
For example, Guid data is hold by some private fields: _a (int); _b and _c (short); _d, _e, _f, _g, _h, _i, _j and _k (byte). So, to binary write a Guid I retrieved its private fields (which are of, let's say, simple types) and values through reflecton and then wrote the data to the stream. Deserializing was the reverse process.
Recursion had to be used when the fields were not of simple types.
It was quite tricky (and funny) to develop that, but after making it work I lost interest on it and, unfortunately, I lost the code as well, but maybe this suggestion can help.
|
|
|
|
|
That might be needed for classes and some structs, but I don't plan on going that far at this time.
What I have now works for Guids and I'll exercise it some more today.
|
|
|
|
|
If you are only doing your own data structures and are willing to use unsafe code. Marshal.StructureToPtr will give you that old school C feel. However I think just using binary reader and writer were faster.
|
|
|
|
|
Andy Brummer wrote: binary reader and writer
That's not read and write.
What I have now supports random access, plus:
[PIEBALD.Attributes.EnumDefaultValue((byte)SupportedType.Unknown)]
public enum SupportedType : byte
{
Unknown
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Byte))]
Byte
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.SByte))]
SByte
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Int16))]
Int16
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.UInt16))]
UInt16
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Int32))]
Int32
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.UInt32))]
UInt32
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Int64))]
Int64
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.UInt64))]
UInt64
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Single))]
Single
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Double))]
Double
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Decimal))]
Decimal
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Guid))]
Guid
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Boolean))]
Boolean
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.DateTime))]
DateTime
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Char))]
Char
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.String))]
String
,
[PIEBALD.Attributes.TypeTransmogrifierAttribute(typeof(System.Enum))]
Enum
}
Oh, and if the type is unknown, it tries the GCHandle technique anyway -- which works for some other types.
modified on Sunday, November 7, 2010 10:54 AM
|
|
|
|