Netduino home hardware projects downloads community

Jump to content


The Netduino forums have been replaced by new forums at community.wildernesslabs.co. This site has been preserved for archival purposes only and the ability to make new accounts or posts has been turned off.
Photo

coding style discussion


  • Please log in to reply
45 replies to this topic

#21 Foozinator

Foozinator

    Member

  • Members
  • PipPip
  • 16 posts

Posted 15 March 2011 - 07:27 PM

Found a reference to overflow checking in the full Framework. http://www.codeproje...w_checking.aspx

#22 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 15 March 2011 - 07:43 PM

I'm not sure what you're getting at. The code example you gave does work (with a few fixes).


I think by changing 'struct' to 'class' you have redefined the question and sidestepped the whole point. You won't be able to get your code to work with 'struct'.

If it helps, here is a self-contained program. The goal is to set the values A and B of array[3] in-place
  • without disturbing the value of C
  • without copying a whole new struct
  • only referring to array[3] inside Func2() once
  • without making any function calls from Func2

Func1 does this, but uses a function call.
Func2 does this, but mentions array[3] twice

You will not be able to rewrite Func2 in a way consistent with the above rules (Redefining MyStruct is beside the point). As I showed above, it however is possible using MSIL, and therefore possible in some hypothetical alternate or future language)

using System;
using System.Diagnostics;

namespace ConsoleApplication13 {
  public class Program {
    public static void Main() {
      var array=new MyStruct[10];
      array[3].C=555;
      Func1(array);
      ShowResults(array);

      array=new MyStruct[10];
      array[3].C=666;
      Func2(array);
      ShowResults(array);
    }

    private static void Func1(MyStruct[] array) {
      Func1Helper(ref array[3]);
    }
    private static void Func1Helper(ref MyStruct s) {
      s.A=13;
      s.B=14;
    }

    private static void Func2(MyStruct[] array) {
      array[3].A=13;
      array[3].B=14;
    }

    private static void ShowResults(MyStruct[] array) {
      Debug.Print("array[3]="+array[3]);
    }
  }

  public struct MyStruct {
    public int A;
    public int B;
    public int C;

    public override string ToString() {
      return string.Format("A={0}, B={1}, C={2}", A, B, C);
    }
  }
}


#23 Foozinator

Foozinator

    Member

  • Members
  • PipPip
  • 16 posts

Posted 15 March 2011 - 07:59 PM

I think by changing 'struct' to 'class' you have redefined the question and sidestepped the whole point. You won't be able to get your code to work with 'struct'.

If it helps, here is a self-contained program. The goal is to set the values A and B of array[3] in-place

  • without disturbing the value of C
  • without copying a whole new struct
  • only referring to array[3] inside Func2() once
  • without making any function calls from Func2


I'm still not sure I understand what you're getting at. Using an interface will satisfy the requirements you listed, I think, but the structs will be boxed. If protecting the value of C is what's important, than either interfaces or better encapsulation looks like a good solution.

		public static void Func2(ABOnly value)
		{
			value.A = 13;
			value.B = 14;
		}

		public static void Func3(MyStruct[] array)
		{
			ABOnly local = array[3];

			local.A = 13;
			local.B = 14;
		}

		public interface ABOnly
		{
			int A;
			int B;
		}

		public struct MyStruct : ABOnly
		{
			public int A;
			public int B;
			public int C;
		}


If you mean accessing the struct without boxing (for performance reasons), I can see your point, but the performance hit from boxing is negligible compared to many other things you find in managed programming and algorithms in general. Unless this is a thought exercise, you should keep in mind that performance tuning is best when surgically applied to specific problem areas.

#24 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 15 March 2011 - 08:14 PM

I'm still not sure I understand what you're getting at. Using an interface will satisfy the requirements you listed, I think,


The interface approach does not work, and in fact your code does not even compile. I think it would help you understand what I am getting at if you try to compile and run your proposed solution using the test program I painstakingly provided. If the output matches what is expected that would be a very interesting result. If not, you can think about alternate approaches. This will help you verify whether your ideas work without having to post proposals to this forum. I don't think this will take very much additional effort, and additionally I think it will bring clarity to the discussion.

#25 Foozinator

Foozinator

    Member

  • Members
  • PipPip
  • 16 posts

Posted 15 March 2011 - 08:32 PM

The interface approach does not work, and in fact your code does not even compile. I think it would help you understand what I am getting at if you try to compile and run your proposed solution using the test program I painstakingly provided. If the output matches what is expected that would be a very interesting result. If not, you can think about alternate approaches. This will help you verify whether your ideas work without having to post proposals to this forum. I don't think this will take very much additional effort, and additionally I think it will bring clarity to the discussion.


Sorry about the compiler errors, but those are easily fixed:

		public static void Func2(ABOnly value)
		{
			value.A = 13;
			value.B = 14;
		}

		public static void Func3(MyStruct[] array)
		{
			ABOnly local = array[3];

			local.A = 13;
			local.B = 14;
		}

		public interface ABOnly
		{
			int A { get; set; }
			int B { get; set; }
		}

		public struct MyStruct : ABOnly
		{
			public int A { get; set; }
			public int B { get; set; }
			public int C { get; set; }
		}

It's better encapsulation to use properties instead of fields, anyway.

More importantly, does this solve the problem?

I'm sorry I don't have the time to retrofit my example into yours. I'm posting from work and they expect me to get stuff done, sometimes. ;)

#26 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 15 March 2011 - 08:34 PM

More importantly, does this solve the problem?


Nope.

#27 Mario Vernari

Mario Vernari

    Advanced Member

  • Members
  • PipPipPip
  • 1768 posts
  • LocationVenezia, Italia

Posted 16 March 2011 - 05:58 AM

Hello friends!...I left you yesterday evening and this morning I have a big headache: OOOOOK! Corey know what is the problem I have raised. Here I explain it again for clarity. First off, all that it popped out playing with Netduino and the NetMF, where there is *few* ram and *slow* speed. On a PC probably none cares. Consider an app, let's call as an "ADC sampler", where a timer fires events every T-ms. Inside the event routine, you have to read the first four ADCs and store them as a "sample-item". The target of the sampler is to fill an array of N sample-items. All that we may call as "sampling cycle". To finish, it would be appreciated that, once a cycle is finished, another one begins after a T_hold time (e.g. 10ms), forever. The data collected from a sampling cycle must be cached for an incoming read of a web server. The only important thing here is that once the cycle has been completed, it must be cloned the samples array in a short time to let the working array free for the next cycle. The first attempt is to define our sampler as follows: class Sample { public int An0; public int An1; public int An2; public int An3; } static class Sampler { static int N = 100; static Sample[] _samples = new Sample[N]; static int _index; // ... void TimerTick(object state) { var item = _samples[_index]; item.An0 = _an0.Read(); item.An1 = _an1.Read(); item.An2 = _an2.Read(); item.An3 = _an3.Read(); } } In terms of RAM, I have noticed that a struct is much more compact than a class. So the array of samples it eats a lot of memory for nothing. Over that, I have tried some experiment and I have seen that a similar approach is very poor performing. By using a struct, the memory is much more compact and I like that, since I really can't waste any byte on a Netduino. The problem of the structs is that they are automatically "cloned" every assignment. So that the following attempt won't work: struct Sample { public int An0; public int An1; public int An2; public int An3; } static class Sampler { static int N = 100; static Sample[] _samples = new Sample[N]; static int _index; // ... void TimerTick(object state) { var item = _samples[_index]; item.An0 = _an0.Read(); item.An1 = _an1.Read(); item.An2 = _an2.Read(); item.An3 = _an3.Read(); } } I should modify the code as follows: void TimerTick(object state) { _samples[_index].An0 = _an0.Read(); _samples[_index].An1 = _an1.Read(); _samples[_index].An2 = _an2.Read(); _samples[_index].An3 = _an3.Read(); } but the performance drops hugely. I have also tried by using some "unsafe" code, but it seems a bit slower (!) than this one: void TimerTick(object state) { Reader(_samples[_index]); } static void Reader(ref Sample item) { item.An0 = _an0.Read(); item.An1 = _an1.Read(); item.An2 = _an2.Read(); item.An3 = _an3.Read(); } This seems the best way for having compactness and speed at same time. And it is safe code too! Would you solve this problem *without* creating a custom routine (i.e. directly inside the TimerTick function)? Corey would point that using the Fluent interop the problem is brillantly solved, but the sense of the post was the "standard" way, using pure C#. Much more a formal discussion than to find a trick to solve the problem off. Cheers Mario
Biggest fault of Netduino? It runs by electricity.

#28 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 16 March 2011 - 12:45 PM

I have also tried by using some "unsafe" code, but it seems a bit slower (!) than this one:

Fascinating. I'm surprised that the unsafe code was slower. I would have thought it would be fine. Can you post the code you wrote? Was it as simple as this:
    void TimerTick(object state) {
      unsafe {
        Sample* sp=&_samples[_index];
        sp->An0=_an0.Read();
        sp->An1=_an1.Read();
        sp->An2=_an2.Read();
        sp->An3=_an3.Read();
      }
    }

But in any case, I'm a little surprised. I would have assumed that the details of this routine are actually minor compared to (1) your event handling overhead (the mechanism that fires the TimerTick in the first place) and (2) the cost of anX.Read(). Is that not the case?

Edited by Corey Kosak, 16 March 2011 - 12:46 PM.


#29 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 16 March 2011 - 12:55 PM

And this is possibly a dangerous coding practice and not guaranteed to work (because the struct may not be laid out the way you think it is), but from an 'experimental' perspective I would love to know if this is any faster:

    void TimerTick(object state) {
      unsafe {
        int* sp=&_samples[_index].An0;
        *sp++=_an0.Read();
        *sp++=_an1.Read();
        *sp++=_an2.Read();
        *sp=_an3.Read();
      }
    }

and likewise I'd like to know if you have been able to get the so-called "fixed buffer" support to work in NETMF, which would allocate an array inline, (I haven't gotten it to work on NETMF):

    private unsafe struct Sample {
      public fixed int An0[4]; 
    }


#30 Mario Vernari

Mario Vernari

    Advanced Member

  • Members
  • PipPipPip
  • 1768 posts
  • LocationVenezia, Italia

Posted 16 March 2011 - 01:29 PM

The unsafe version that I've used is here, but is the same as your's:
http://stackoverflow...o-improve-speed
Isn't a good choice: has the same speed of the safer way and it's dangerous too.

Anyway the Netduino world is driving me crazy. I *must* use some pattern to improve speed and sparing memory, but I'm also realizing all the beauty of C# is losing in favor of C++ patterns.
I'm a bit scared about using structs, because often are very tricky.
Read this:
http://blogs.msdn.co...ly-structs.aspx

At this point, for my problem, I see only three choices:
  • custom hardware
  • Fluent interop
  • custom driver C++
sorted by preference.

I'll post some considerations about Fluent on a separate thread.
The C++ driver is the worst case, because I don't know C++ and I would not even recompile firmware. I have not experiences of gcc or similar, so the probability to hang my pc is near 100%.

For me is relatively easy to play with the hardware, so maybe...
Bah!...
Biggest fault of Netduino? It runs by electricity.

#31 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 16 March 2011 - 02:59 PM

Anyway the Netduino world is driving me crazy.


I feel your pain. And also, you're right, my code is broken because it doesn't use "fixed" on the pointer. Oops :wacko: Well, that's easily fixed, but if it doesn't help very much it doesn't matter. Anyway, I'll address your fluent questions on the other thread.

Edited by Corey Kosak, 16 March 2011 - 03:29 PM.


#32 AlfredBr

AlfredBr

    Advanced Member

  • Members
  • PipPipPip
  • 138 posts
  • LocationConnecticut, USA

Posted 16 March 2011 - 03:59 PM

	public class Program
	{
		public static void Main()
		{
			var count = GetValue();

			count += 200;
		}

		public static byte GetValue()
		{
			return 100;
		}
	}

Let's assume that GetValue is actually buried somewhere in another file. If the developer is expecting that GetValue returns an int, he won't expect to see count = 44 after that second line.

If count is explicitly defined as an int, it will behave as the developer expects (implicit cast to a larger type).

Also, if the types were reversed (GetValue returns an int, but count is defined as a byte), you'll see the compiler error "Cannot implicitly convert type 'int' to 'byte'. An explicit conversion exists (are you missing a cast?)". (Yes, I know that declaring count as var here won't make a difference to the compiler, as it already knows to use int.)



Looks to me that the caller of GetValue() made an error (when declaring his local variable), not that var assumed the wrong type returned from GetValue().

#33 Foozinator

Foozinator

    Member

  • Members
  • PipPip
  • 16 posts

Posted 16 March 2011 - 07:32 PM

Looks to me that the caller of GetValue() made an error (when declaring his local variable), not that var assumed the wrong type returned from GetValue().


My point was that error (incorrect assumption) doesn't show up as a compile error in this case, where if the developer used an explicit type instead of var, there would be either a compile error or, in this case, no problem at all.

#34 Chris Seto

Chris Seto

    Advanced Member

  • Members
  • PipPipPip
  • 405 posts

Posted 16 March 2011 - 11:59 PM

I think a lot of this sort of discussion is a bit of a lost cause.

Do yourself a favor: Buy a copy of Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries (2nd Edition)

http://www.amazon.co...BD3Q166WETW2H4F

Read it, and apply those patterns. :)

Also, I highly despise the use of "var" in code. Just felt like I should say that.... ;)

#35 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 17 March 2011 - 12:44 AM

Well, I find these discussions kind of engaging. If you can reveal your reasons why you think the discussion is a lost cause, or why you think var is despicable, it might be interesting to hear them.

#36 Illishar

Illishar

    Advanced Member

  • Members
  • PipPipPip
  • 146 posts

Posted 25 March 2011 - 07:53 AM

As a experienced pro programmer, I'd like to join the group that despises the "var" keyword. It might be compile time typesafe, but it's still an abomination. (Among others it makes the code less readable I think)

#37 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 25 March 2011 - 12:53 PM

As a experienced pro programmer, I'd like to join the group that despises the "var" keyword. It might be compile time typesafe, but it's still an abomination.

(Among others it makes the code less readable I think)

I'd like to respond to this, but I'm finding it difficult to do so because I'm not sure what you mean by "abomination". I think think of four kinds of situations involving 'var':
  • Situations where it is absolutely required (there is no way to avoid it)
  • Where it can be avoided if one is willing to bring in (likely irrelevant) details from the called routine
  • Where one would otherwise type literally the same sequence of characters twice
  • (other situations)
As a regular human being (although I do happen to also be an experienced pro programmer) I have to think that points 1-3 are so uncontroversial that anyone else would have to feel the same way. If not, the consequences of point 1 are that if you get rid of "var" you have to throw away parts of LINQ. Maybe you're OK with that (I'm not), but in any case without more detailed information about your point of view, I have to believe that when you say "despise" and "abomination" you really only refer to the subset of situations referred to in point 4.

Examples for 1-3:

Item 1:
      IEnumerable<People> people=GetPeople();

      var peopleWithSameNames=from person in people
                              group person by new {person.LastName, person.FirstName}
                              into g
                              select new {g.Key.LastName, g.Key.FirstName, Count=g.Count()};

Items 2 and 3:
    static void Main(string[] args) {
      Dictionary<Tuple<string,string>, List<int>> dict1=new Dictionary<Tuple<string, string>, List<int>>(); //item 3
      Dictionary<Tuple<string,string>, List<int>> dict2=new Dictionary<Tuple<string, string>, List<int>>(); //item 3

      Dictionary<Tuple<string, string>, List<int>>.KeyCollection dict1Keys=dict1.Keys; //item 2
      Dictionary<Tuple<string, string>, List<int>>.KeyCollection dict2Keys=dict1.Keys; //item 2

      IEnumerable<Tuple<string, string>> allKeys=dict1Keys.Concat(dict2.Keys);
    }

In a world where var is not despised, this looks like:

    static void Main(string[] args) {
      var dict1=new Dictionary<Tuple<string, string>, List<int>>(); //item 3
      var dict2=new Dictionary<Tuple<string, string>, List<int>>(); //item 3

      var dict1Keys=dict1.Keys; //item 2
      var dict2Keys=dict1.Keys; //item 2

      var allKeys=dict1Keys.Concat(dict2.Keys);
    }


#38 Nevyn

Nevyn

    Advanced Member

  • Members
  • PipPipPip
  • 1072 posts
  • LocationNorth Yorkshire, UK

Posted 25 March 2011 - 01:13 PM

Item 1:

      IEnumerable<People> people=GetPeople();

      var peopleWithSameNames=from person in people
                              group person by new {person.LastName, person.FirstName}
                              into g
                              select new {g.Key.LastName, g.Key.FirstName, Count=g.Count()};


I believe that this can be avoided by creating a new class to hold the names and create a new instance of the class. That way the var can be replaced by an IEnumerable of the new class.

    static void Main(string[] args) {
      Dictionary<Tuple<string,string>, List<int>> dict1=new Dictionary<Tuple<string, string>, List<int>>(); //item 3
      Dictionary<Tuple<string,string>, List<int>> dict2=new Dictionary<Tuple<string, string>, List<int>>(); //item 3

      Dictionary<Tuple<string, string>, List<int>>.KeyCollection dict1Keys=dict1.Keys; //item 2
      Dictionary<Tuple<string, string>, List<int>>.KeyCollection dict2Keys=dict1.Keys; //item 2

      IEnumerable<Tuple<string, string>> allKeys=dict1Keys.Concat(dict2.Keys);
    }


In many of the companies I have worked for this breaks local coding standards as we were not allowed to declare a variable and instantiate it on the same line. I think the idea was to leave space for a comment on the purpose of the variable if it's name or use was not clear.

I'm on the anti-var camp and so far the only argument I have heard for it which may convince me of its value is the functional programming argument - and this is something I would need to do further research on before I could make a comment with any authority.

Regards,
Mark

To be or not to be = 0xFF

 

Blogging about Netduino, .NET, STM8S and STM32 and generally waffling on about life

Follow @nevynuk on Twitter


#39 Corey Kosak

Corey Kosak

    Advanced Member

  • Members
  • PipPipPip
  • 276 posts
  • LocationHoboken, NJ

Posted 25 March 2011 - 02:09 PM

I believe that this can be avoided by creating a new class to hold the names and create a new instance of the class. That way the var can be replaced by an IEnumerable of the new class.

That's correct -- so long as it has a proper implementation of Equals() and GetHashCode(), which turns out to be a quite a bit of typing. You have to be a bit of an expert just to get .Equals() right.

Then if you need to change the class, you need to decide whether you are its only user (in which case you can modify it) or whether there are other parts of your codebase relying on this "helper" class (in which case you need to make a new one). And then, surely your coding style rules say that you should get rid of classes that are no longer used, so you may need to delete old ones. It would be interesting to hear your/your company's take on this argument, as I would assume their interest is in minimizing the amount of code they need to test and maintain. i.e. why not let the compiler automatically supply these classes and their methods rather than put your programmers through all this busywork?

In other words, there's no creativity (and therefore no value-add) involved in writing such classes (it's all boilerplate), but there certainly is plenty of pain on the downside when one makes a mistake and gets them wrong.

In many of the companies I have worked for this breaks local coding standards as we were not allowed to declare a variable and instantiate it on the same line

I'm pretty surprised, as this would open the door to uninitialized variable bugs in C/C++. The compiler won't allow such bugs in C#/Java but still I don't see why you'd want there to be a gap between a variable's declaration and its initialization. The desire to leave space for commenting does not strike me as particularly compelling, as there are lots of ways to format code which allow for copious commenting.

Just the other night as I was trying to write my stack-machine-to-register-machine translator for SimpleNGEN, I found myself writing this line:
      var incomingStacks=new Dictionary<ControlFlowGraphNode<Labeled<StackOperation>>, MyNode<AbstractRegister>>();
I think I have difficulty respecting the IT manager who would insist that this is better as
      Dictionary<ControlFlowGraphNode<Labeled<StackOperation>>, MyNode<AbstractRegister>> incomingStacks;
      incomingStacks=new Dictionary<ControlFlowGraphNode<Labeled<StackOperation>>, MyNode<AbstractRegister>>();
I mean, ugh, really? Does that clarify or obfuscate?

#40 Mario Vernari

Mario Vernari

    Advanced Member

  • Members
  • PipPipPip
  • 1768 posts
  • LocationVenezia, Italia

Posted 25 March 2011 - 02:30 PM

Although that is an interesting match, I think we're going off the real problems of Netduino and the MF. However, happy to have discussion with anyone wants.
Just FYI, a couple of years ago, I have found this document. I think it is very well done, even you may don't agree though.
http://csharpguidelines.codeplex.com/
In particular for the "var" question, it says (page 15):

Only use var when the type is very obvious
Only use var as the result of a LINQ query, or if the type is very obvious from the same statement and using it would improve readability.
Don't

var i = 3; // what type? int? uint? float?
var imyfoo = MyFactoryMethod.Create("arg"); // Not obvious what base-class or
// interface to expect. Also difficult
// to refactor if you can't search for
// the class
Do:
var q = from order in orders where order.Items > 10 and order.TotalValue > 1000;
var repository = new RepositoryFactory.Get<IOrderRepository>();
var list = new ReadOnlyCollection<string>();
In all of three above examples it is clear what type to expect.

Which I agree at all, even my habit is to use "var" often.
All that sounds much more like "the signal says don't run over 50KM/h, but there is none around, so I'll run faster".

I think every developer should adapt the most reasonable rules can do to make the code readable and easily to manage.
Biggest fault of Netduino? It runs by electricity.




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users

home    hardware    projects    downloads    community    where to buy    contact Copyright © 2016 Wilderness Labs Inc.  |  Legal   |   CC BY-SA
This webpage is licensed under a Creative Commons Attribution-ShareAlike License.