Meteor

16 hours ago, vesuvius wrote

*snip*

that code is 30 years old, things have definitely improved, so not really worth spending too much time on apart from historic curiosity.

With that response, Sir, you embodied Microsoft's problems here and now!

@fanbaby: I doubt any of those guys would refer to that as beautiful code. Turns out we've all learned a lot about writing more maintainable code since then. Remember C started out as little more than portable assembly, so it's unsurprising it reads like it too.

@fanbaby: I doubt any of those guys would refer to that as beautiful code. Turns out we've all learned a lot about writing more maintainable code since then. Remember C started out as little more than portable assembly, so it's unsurprising it reads like it too.

@Dr Herbie: Oh, sorry for the late reply

The code wouldnt pass because it has multiple exits in one method. Our coding guidelines state that one method should have one return.

@MasterPie: That's what the contract namespace is for! (in .Net)

14 hours ago, Bas wrote

*snip*

I do the same thing. I know it's technically more correct to have a single point of return in a method, but I find that my code becomes much more readable if I just have multiple returns instead of a bunch of nested conditions and checking variables. I prefer readability over.. beauty, or whatever you'd call it, so multiple returns it is.

Its not technically more correct, it is encapsulation, a fundamental principle in object-oriented development. It may be clearer to you, but if you have to work on a system riddled with ref and out parameters it is a bit like goto with methods, so the only way someone new looking at your code can understand it, is by debugging, especially if you are dealing with encoders or decoders or custom protocol documents, you want to be as clear as possible because typically that code is very complex anyway.

I have had this debate with various teams, and the people who write code with ref and out parameters are always leave their teammates irritated. Encapsulation, all the way!

@vesuvius: I think there is confusion here over multiple points of return and multiple return values.

I think what Bas was talking about was ducking out of a method early rather than setting a variable and processing the rest of the method with a set of 'if(weNeedToCarryOnProcessing)' conditions to get the the single point of return at the bottom of the method.

Herbie

18 minutes ago, Dr Herbie wrote

@vesuvius: I think there is confusion here over multiple points of return and multiple return values.

I think what Bas was talking about was ducking out of a method early rather than setting a variable and processing the rest of the method with a set of 'if(weNeedToCarryOnProcessing)' conditions to get the the single point of return at the bottom of the method.

Herbie

Uh-huh, I agree then, but it's 50/50 because some people just prefer the other way.

Why are you guys criticising one piece of sample code, and going way off topic?  It's like a finger pointing the way to the moon.  Don't look at the finger, or you'll miss all the heavenly glory! (Bruce Lee)

 It's a sample, simplified for presentation purposes.  If you really want to pick on the code, pick on Meteor's code base - not presentation samples -  and fix it while you are at it, the way you say it should be. 

@Richard.Hein:It turned into a discussion of what people look for in code reviews -- yes it's off-topic, but it's interesting. Could have spawned a new thread but ... meh.

Herbie

@Maddus Mattus:Aw come on. Not everyone has the luxury to use .NET. And besides, I found that even the released ASP.NET MVC source checks for invariants at the beginning of the method.

Edit: nevermind, I was getting confused....

47 minutes ago, blowdart wrote

*snip*

Different thing, but lets address that. It can be quite common. For example TryParse. I have no problem with a method which returns a status code, and takes in a ref which may be set, or not, depending on what the status code returned is.

That is fine, a swap function is another very good case for using ref parameters 

 public static class Reference {
   public void Swap<T>(ref T obj1, ref T obj2){
      T temp = obj1;
      obj1 = obj2;
      obj2 = temp;
   }
}

 to quote Anders Heijlsberg; "As a rule, I am not too crazy about ref and out parameters in APIs. Such APIs compose very poorly, forcing you to declare temporary variables. I much prefer functional designs that convey the entire result in the return value."

to quote Brian Pepin; If you need to return several pieces of data from a call, wrap that data up into a class or struct. For example, the .NET Framework has an API to perform hit testing of controls that returns a HitTestResult object.

to quote Chris Sells; Swap always comes up in these discussions, but I have not written code that actually needed a swap function since college. Unless you've got a very good reason, avoid out and ref altogether

In all this guidance no-one uses the word never, as there will be rare cases where use is appropriate, I mean iterators in C# have a goto in them. I recently reviewed code with a method signature with loads of parameters using out that would have best been served with encapsulation, I come across people doing this from time to time.

Source for quotes is here.

@MasterPie: That's exactly what the Contracts namespace does, it generates the code for you.

No .Net?

I pity the fool who has no .Net!

I even code my javascript in C# .Net! (And then compile it to javascript, http://projects.nikhilk.net/ScriptSharp FTW!)

Not sure it supports Contracts tho.

@Maddus Mattus:Wait, so it ultimately generates those same checks at the beginning of methods? Then how exactly is it bad if you write them yourself?

26 minutes ago, MasterPie wrote

@Maddus Mattus:Wait, so it ultimately generates those same checks at the beginning of methods? Then how exactly is it bad if you write them yourself?

Reinventing the wheel, and if you look at the implementation, some pretty smart researchers have gone through the hassle of ensuring you don't have to worry about making a hash of things, I think the unavailability of contracts in professional and express versions has affected their use

@MasterPie: It's a fallback mechanism.

I don't know exactly, but here is what I think (guru's correct me if I am wrong);

I believe it was F# that introduced contracts. There they actually form a barrier between the caller and the object called. It's part of the language (or the compiler, or something, F# not my cup of tea).

As C# and VB.Net don't have this feature in the language(...), Contracts fall back on code generation. They are much easyer to code and read then all those if statements.

Contract.Assert(someparam => someparm != "somevalue", "Invalid someparam!");

v.s.

if(someparam != "somevalue")
{
throw new InvalidArgumentException("someparam");
}

Now imagine 5 of those statements.