|
Depends - if I can reasonably name the variables "almost like" the argument names (e.g. "targetReceptor" instead of "target") and if I have no reason to use an expression as parameter then I go for the second form because I don't feel the first form would increase the clarity of the code.
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
The first sort of feels clunky but it is much safer.
Someone will one day insert a field in the middle of the others and the thing will go kaboom(I hope I am not being too technical in my language here).
“That which can be asserted without evidence, can be dismissed without evidence.”
― Christopher Hitchens
|
|
|
|
|
GuyThiebaut wrote: insert a field in the middle of the others and the thing will go kaboom
Good point. 
|
|
|
|
|
|
As always, the only valid answer is: it depends.
Does it make sense for the properties to be writeable? Particularly in an EventArgs class, where you probably don't want one rogue handler to change the data that other handlers see.
If you have lots of parameters, and you don't always need to pass all of them, an object initializer might be cleaner. But you could do something similar with optional parameters, or provide constructor overloads for each valid set of parameters. And if you're in that situation, it might be time to refactor the class anyway, because it's probably breaking the SRP.
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Richard Deeming wrote: Does it make sense for the properties to be writeable?
5! Yes, that's the hidden "gotcha". When using a constructor, the class can be initialized like this:
public class ProcessEventArgs : EventArgs
{
public IMembrane FromMembrane { get; protected set; }
public IReceptor FromReceptor { get; protected set; }
public IMembrane ToMembrane { get; protected set; }
public IReceptor ToReceptor { get; protected set; }
public ISemanticType SemanticType { get; protected set; }
public ProcessEventArgs(IMembrane fromMembrane, IReceptor fromReceptor, IMembrane toMembrane, IReceptor toReceptor, ISemanticType st)
{
FromMembrane = fromMembrane;
FromReceptor = fromReceptor;
ToMembrane = toMembrane;
ToReceptor = toReceptor;
SemanticType = st;
}
}
Note the protected setters.
|
|
|
|
|
Or better yet, like this:
public class ProcessEventArgs : EventArgs
{
public IMembrane FromMembrane { get; }
public IReceptor FromReceptor { get; }
public IMembrane ToMembrane { get; }
public IReceptor ToReceptor { get; }
public ISemanticType SemanticType { get; }
public ProcessEventArgs(IMembrane fromMembrane, IReceptor fromReceptor, IMembrane toMembrane, IReceptor toReceptor, ISemanticType semanticType)
{
FromMembrane = fromMembrane;
FromReceptor = fromReceptor;
ToMembrane = toMembrane;
ToReceptor = toReceptor;
SemanticType = semanticType;
}
}
(Assuming you're using the C# 6 compiler or later. Otherwise, private set; would have a similar effect.)
Also, named parameters can make the constructor look more like the object initializer:
Processing.Fire(this, new ProcessEventArgs(
fromMembrane: fromMembrane,
fromReceptor: fromReceptor,
toMembrane: membrane,
toReceptor: target,
semanticType: obj
));
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
modified 13-Oct-17 10:32am.
|
|
|
|
|
Richard Deeming wrote: Also, named parameters can make the constructor look more like the object initializer:
Ah, I've never used that syntax in C#. And yes, omitting the setter or marking it private too. 
|
|
|
|
|
Richard Deeming wrote: Also, named parameters can make the constructor look more like the object initializer:
Now that I think about it, I have used that syntax, in ASP.NET/Razor. Personally, I think the only reason they added that form of initialization was so that people used to JSON object initialization in JavaScript would be comfortable. 
|
|
|
|
|
The answer, of course, is that it all depends. The second example has the potential to introduce errors that the first doesn't if you're looking at validation to make sure that the values in ProcessEventArgs aren't null (validating them in the constructor of course). More importantly, the second example implies that you have opened up the scope of the properties to be read/write as opposed to read only. It really depends on what the use case is for ProcessEventArgs.
This space for rent
|
|
|
|
|
Pete O'Hanlon wrote: More importantly, the second example implies that you have opened up the scope of the properties to be read/write as opposed to read only
5! Yup.
|
|
|
|
|
I'm old school, so the second one any day! 
Anything that is unrelated to elephants is irrelephant
Anonymous
-
The problem with quotes on the internet is that you can never tell if they're genuine
Winston Churchill, 1944
-
I'd just like a chance to prove that money can't make me happy.
Me, all the time
|
|
|
|
|
Johnny J. wrote: I'm old school, so the second one any day!
Hey there's this new fancy language called COBOL you should check out. 
Jeremy Falcon
|
|
|
|
|
Too soon. Gotta wait a bit and see if it catches on... 
Anything that is unrelated to elephants is irrelephant
Anonymous
-
The problem with quotes on the internet is that you can never tell if they're genuine
Winston Churchill, 1944
-
I'd just like a chance to prove that money can't make me happy.
Me, all the time
|
|
|
|
|
|
Jeremy Falcon
|
|
|
|
|
Arriving late to the party (comments indicate multiple edits have been made).
For my older eyes, I prefer the first; I can see what the arguments are on each line without having to scan a list looking for something.
|
|
|
|
|
Tim Carmichael wrote: I prefer the first; I can see what the arguments are on each line without having to scan a list looking for something.
Me too, but as others have pointed out, it allows for the properties to be write-able.
|
|
|
|
|
Clarification...
I prefer the properties to be listed, one per line, as in option 1 versus having them all listed on a single line.
|
|
|
|
|
Parameters that are required for establishing invariant should be part of constructor. For others, do as you're pleased. I prefer #2, but I also like read-only field/properties, but that subject is already covered by others in this thread.
|
|
|
|
|
For me, it depends. If it's just a few params I tend to go for the latter since a couple params isn't hard to remember. If you end up with a constructor or method that takes a crap ton of them then I use the former always so there's no guesswork as to what the params are.
For me, code is like art. You make it look pretty and readable on a case-by-case basis. If it makes sense to do something then you do it, but it doesn't always mean you do the same thing every time for the rest of your life. It's on a case-by-case basis.
Jeremy Falcon
|
|
|
|
|
Jeremy Falcon wrote: For me, code is like art. You make it look pretty and readable on a case-by-case basis. If it makes sense to do something then you do it, but it doesn't always mean you do the same thing every time for the rest of your life.
Whenever I make my code pretty I invariably have to revisit it one more time to make it functional.
|
|
|
|
|
The former, even if only because the list of args provided can keep growing and it'll remain readable. Whereas in the latter form, you've probably already reached the limit of what you can see without scrolling horizontally.
If that's the dumbest reason imaginable, then I'll still insist it gives me the ability to merely glance at the code to figure out what it's doing.
|
|
|
|
|
Marc Clifton wrote: Which form do you prefer? Why?
The first. Because Ctrl-X.
Bastard Programmer from Hell 
If you can't read my code, try converting it here[^]
|
|
|
|
|
I prefer the second. Because:
- When reviewing code (someone else, or you 6 months later), the idea of what is happening can be seen better from a distance. It is clear what is happening on that one line versus having to parse 8 lines to understand what is happening (we're talking human parsing here). The "verb" is on the left, the "nouns" on the right (well okay, there are 2 things going on here. You are building a thing then using it). [I try to do only one thing per line ... not two or more actions per line and not two or more lines per action... see below.]
- There is less code, which is simpler (duh), statistically more reliable and easier to maintain. I like to write (and read) code two dimensionally so that it fills more across the page and is more natural to read by humans. Compare:
(1) This is easy to read.
(2) This
is
harder
to
read
Bugs in code tend to occur at a relatively fixed number per thousand lines of code... one of the reasons I always try to use the highest level language possible, and don't violate this previous point.
How I would do it
auto pea = new ProcessEventArgs(fromMembrane, fromReceptor, membrane, target, obj);
Processing.Fire(this, pea);
Yeah, you're creating another variable, but the compiler knows what to do with it, and the code offers something that the debugger can help you with. This code is more debuggable.
I'm retired. There's a nap for that...
- Harvey
|
|
|
|
|
Neither. I'm a white-space supremacist.
Processing.Fire
(
this
,
new ProcessEventArgs
(
fromMembrane
,
fromReceptor
,
membrane
,
target
,
obj
)
) ;
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
