I wrote a code comment today

There is never a good time for a single massive method instead of several smaller methods. A single giant method is harder to debug. At a minimum, you could break the single large function into several smaller well named methods and call those smaller methods from your previously large method. The method calls would replace the comment headers. It makes it easier to modify or debug your code, especially if you come back to it a few months later.

Clean code is one of the greatest gifts you can give yourself. A single massive function will create extra work for your future self.

 

Again, if we use Ander's example - why do you think the single method is worse?

If we move out of the abstract, and look at specific examples, do these kinds of 'golden rules' stand to scrutiny, or is this something closer to adhering to dogma as opposed to rationality?

In your response I see a lot of normative statements, "good" "clean" - I don't see much substance.

 

Who?

 

this thread: Oxford commas.

 

How do you guyts do state management. BLoc, redux, mobX ... provider. It's quite confusing for me? I'm dying inside when I look at redux.

Spoiler: spoiler

 

Have you had the benefit to dissect someone's large method? Because most people I have come across finds its easier when it's devided into logical modules of some sort.

 

I think the problem is that most of the time, people who write big methods are newbies, so people think big methods are guilty by association.

At a high level, I kind of use a simple mental checklist:

• Does a method represent a strict sequence of actions (is it a process with a clean start/end or similar to a traditional algorithm)?
• Are the steps of the process very specific to the process (ie: non-generalized). Am I unlikely to use the code elsewhere?
• Can I write the method inline without repeating myself. Can the statements be written inline without lots of duplication?

If the answer to all of these are yes, I tend to think that when the code itself looks similar to how it behaves (a strict sequence in processing, being represented in code as a strict sequence of statements) - it's easier to read, understand and modify.

I think your snippit is an example where the answer to all 3 of those questions is yes. So I would argue that a single, 70 line method is a cleaner implementation than 7-9 separate methods.

That said, micro methods are far easier to write automated tests for. So if test coverage is a major concern, I can see the argument for micro methods to enable easier unit tests. One could look at my checklist and say that testing could be seen as an example of 'another place where the code needs to be reused', and therefore not a good candidate to be written all inline.

 

Yeah, like how most gamers overclock their computers :v

 

And the last and most important point, can I easy get a overview of the flow. If you can't you should probably break it down to smaller chunks, just like I did in the example.

 

I still use Oxford commas, and I probably always will.

 

I use them, abuse them, and never refuse them.

 

I have over 30 years of programming experience. My comments regarding the use of comments in code are my opinions based on what I have learned over time. In my experience, a single massive method always creates more work for me down the road. That has been the case in every project in every language over time.

If a single method is so long that you are considering adding comment headers to break up the code, then you should take the additional step of splitting up the code into multiple methods. If you are concerned with the performance overhead of passing the parameters around, then simply store those parameters at the class level. But definitely split the single long method into several smaller methods.

 

I understand where you are coming from on #2 and #3, but that may or may not need to be part of the criteria. For example, the answer to #2 is often unknown early in the project. There may be code you could re-use if it was in its own method, but you might not realize it initially. Then after you move that code into its own method, you may actually find several times to re-use it.

And with #3, remember it is not your job as a coder to manually inline things. Modern compilers can automatically inline code. There are still some extremely rare cases where a programmer might need to manually inline code to build a super tight loop in code that runs on the metal, but that is not relevant to any coding we would do in C# for a game in Unity. Don't manually inline code.

 

Today, I plan to conquer SM .

 

The call to

Code (CSharp):

1. script.Execute(setting.Script, new Dictionary<string, object> {{"request", request}});

Is actually invoking iron python a perfect script language in my opinion, easy for our testers to modify without large amount of prior coding experience.

 

TBH that's how it should be, but I guess the closer to the metal you get the more verbose it gets.

I adore python !!! below was my first ever entry level python script. . .

Code (CSharp):

1. #Iamthwee's Hello World,
2. #Hello World,Bonjour tout le monde, Hallo Welt, Herro Word - American/French/German/Engrish
3.  
4. #Quad Licensed under the GPLv3/BSD/Python/CC-BY-NC-SA 2.0
5. #THIS PROGRAM COMES WITH NO WARRANTY, BECAUSE I'M TOO STRESSED TO TROUBLESHOOT YOUR PROBLEMS!!!
6. x = 3
7. y = 5
8. if x + y = 8:
9.     import sys
10.     from sys import *
11.     import os
12.     from os import *
13.     import PySide
14.     from PySide import *
15.     import PySide.QtCore
16.     import PySide.QtNetwork
17.     import cmath
18.     import numpy
19.     import pypolynomial
20.     from pypolynomial import *
21.     import twisted
22.     from twisted import *
23.     import math
24.     from math import *
25.  
26.       while x = 3:
27.            
28.             class printerunitui(QtGui.QWidget)
29.  
30.  
31.                       def __init__(self):
32.                    
33.                          if __name__=__main__:
34.                      
35.                            while y = 5:
36.                          
37.                                 for 'x' = 'x':
38.                                  
39.                                      QtGui.QWidget.__init__(self)
40.      
41.                                                                   self.setWindowTitle('Printerunit')
42.  
43.  
44.  
45.  
46.  
47.                        def metaprinterconsoleunit():
48.  
49.                                                      inval = input('Requires input')
50.                                                      
51.                                                            if inval = 'yes':
52.                      
53.                                                             messagestringconstantglobalvalue = 'Hello World'
54.                                                        
55.                                                             print(messagestringconstantglobalvalue)
56.        
57.                                                            else:
58.                                
59.                                                                    print('Fatal error')
60.                    
61.                             exit()
62.  
63.  
64.                     printerunitui()
65.                     metaprinterconsoleunit().exec()
66.       sys.exit()

Flutter looks like callback hell

Code (JavaScript):

1. import 'package:flutter/material.dart';
2.  
3. void main() => runApp(MyApp());
4.  
5. class MyApp extends StatelessWidget {
6.   // This widget is the root of your application.
7.   @override
8.   Widget build(BuildContext context) {
9.     return MaterialApp(
10.       title: 'Flutter Demo',
11.       theme: ThemeData(
12.         primarySwatch: Colors.grey,
13.       ),
14.       home: MyHomePage(),
15.     );
16.   }
17. }
18.  
19. class MyHomePage extends StatefulWidget {
20.   @override
21.   _State1 createState() => new _State1();
22. }
23.  
24. class _State1 extends State<MyHomePage> {
25.   int _counter = 0;
26.   int _counter2 = 0;
27.  
28.   void _incrementc() {
29.     setState(() {
30.       _counter++;
31.     });
32.   }
33.  
34.   void _incrementd() {
35.     setState(() {
36.       _counter2++;
37.     });
38.   }
39.  
40.   @override
41.   Widget build(BuildContext context) {
42.     return Scaffold(
43.       appBar: AppBar(
44.         title: Text("Title"),
45.         elevation: 1.0,
46.       ),
47.       body: Center(
48.         child: ListView(
49.           children: <Widget>[
50.             Container(
51.               padding: const EdgeInsets.all(30.0),
52.               child: Text(
53.                 '$_counter',
54.                 style: TextStyle(
55.                   fontSize: 20.0,
56.                 ),
57.                 textAlign: TextAlign.center,
58.               ), //end text
59.             ), //end container
60.  
61.             Container(
62.               padding: const EdgeInsets.all(30.0),
63.               child: Text(
64.                 '$_counter2',
65.                 style: TextStyle(
66.                   fontSize: 20.0,
67.                 ),
68.                 textAlign: TextAlign.center,
69.               ), //end text
70.             ), //end container
71.  
72.             Incrementer(_incrementc),
73.             Incrementer(_incrementd),
74.           ],
75.         ), //end listview
76.       ), //end center
77.     ); //end scaffold
78.   } //end builder
79. } //end class
80.  
81. class Incrementer extends StatefulWidget {
82.   final Function incrementc;
83.  
84.   Incrementer(this.incrementc);
85.  
86.   @override
87.   _StateIncremeter createState() => new _StateIncremeter();
88. }
89.  
90. class _StateIncremeter extends State<Incrementer> {
91.   @override
92.   Widget build(BuildContext context) {
93.     return Container(
94.       padding: const EdgeInsets.only(left: 20.0, right: 20.0, top: 20.0),
95.       child: RaisedButton(
96.         color: const Color(0xff1667D1),
97.         textColor: Colors.white,
98.         shape: new RoundedRectangleBorder(
99.             borderRadius: new BorderRadius.circular(30.0)),
100.         onPressed: widget.incrementc,
101.         child: Text("Increment"),
102.       ), //end raised button
103.     ); //end container
104.   } //end builder
105. } //end class
106.  

 

Compiler supported async programming in c# is nice!

 

I find flutter a bit tedious to understand http async POST/GET/PUT requests but I guess after a bit of practice it becomes easier e.g

Code (JavaScript):

1.  
2.  
3. Future<String> getHttp(email, password) async {
4.     try {
5.  
6.  
7.       Response response = await Dio().post(
8.         "https://somedomain.com",
9.         data: {"email": email, "password": password},
10.        
11.        
12.       );
13.  
14.       print(response.data.toString());
15.  
16.       Map<String, dynamic> map = response.data; // import 'dart:convert';
17.  
18.       String toke = map['token'];
19.       int userid = map['user']['CustomerUserId'];
20.  
21.  
22.       this.setState(() {
23.         result = userid.toString();
24.         _save(token);
25.       });
26.  
27.       //----------------------------------------------
28.       //
29.       //
30.       //   If email and password successful
31.       //   go to the dashboard screen
32.       //
33.       //-----------------------------------------------
34.  
35.       Navigator.pushNamed(context, '/dashboard');
36.  
37.     } catch (e) {
38.       print('login failed');
39.  
40.       this.setState(() {
41.         result = 'Login failed check email and passwords!';
42.       });
43.       _showAlert(context, result);
44.     }
45.   }

 

@AndersMalmgren I use node -- adonisjs for my backend restful API as it is faster than dotnet under heavy usage and is actually a joy to read and write IMO, it's dead easy to authenticate JWT (json web tokens) to boot!

https://adonisjs.com/docs/4.1/api-tests

 

How do you boys do unit tests in dotnet?

 

Here you and I simply differ. I've also got decades of experience writing code, and more recently I've found that refactoring single methods into various submodules dogmatically can produce inferior code.

The reason being that reading code in sequence, from top to bottom, is closer to natural human language. Being able to see the specific mutations, all written inline, gives a more complete sense of what's being done and prevents misinterpretation.

Code (csharp):

1.  
2.             //Check if a setting exists for a request
3.             var queryInPath = false;
4.             var setting = settings.FirstOrDefault(s =>
5.             {
6.                 queryInPath = s.Path.IndexOf("{", StringComparison.Ordinal) >= 0;
7.                 return ctx.Request.Method.ToLower() == s.Method.ToLower() && CheckSegments(s.Path.Split('/'), ctx.Request.Path.Value.Split('/'));
8.             });
9.             if (setting == null)
10.             {
11.                 await _next(ctx);
12.                 return;
13.             }
14.             //Create a request DTO
15.             object body = null;
16.             var contentTypeDelim = ctx.Request.ContentType?.IndexOf(";", StringComparison.Ordinal);
17.             var strippedContentType = contentTypeDelim.HasValue && contentTypeDelim >= 0 ? ctx.Request.ContentType.Substring(0, contentTypeDelim.Value) : ctx.Request.ContentType;
18.             if (ctx.Request.Method == "POST" && BodyFactory.ContainsKey(strippedContentType) && ctx.Request.ContentLength > 0)
19.             {
20.                 body = await BodyFactory[strippedContentType](ctx.Request);
21.             }
22.             var request = new Request
23.             {
24.                 Headers = ctx.Request.Headers.ToDictionary(h => h.Key, h => h.Value.ToString()),
25.                 QueryParams = ctx.Request.Query.ToDictionary(q => q.Key, q => q.Value.ToString()),
26.                 Body = body
27.             };
28.  

Again, this is a great example. In one solid block I'm able to see what's being done and why it's being done. If I am looking at this code for the first time, it may look more intimidating at first, but by actually reading through it, I can get a more complete view of whats happening in clearer terms.

This may simply be a difference in how your brain reads code and how my brain reads code, and that's fine. But this is not a black and white issue of "clean" code vs "bad" code.

I would agree with both your points. But if the answer to #2 is "not now" then YAGNI. If later in the dev process you realize that elements should be generalized, refactor it when the need is clear.

In terms of #3, I'm not talking about inlining for performance gain, I'm talking about keeping all the relevant statements in a single method for additional clarity when reading.

I've worked plenty with both methods, and experimented with keeping code that follows those 3 rough guidelines as single methods, I've found that they're easier to work with and clearer in actual practice. Especially when dealing with particularly complex or nuanced code.

One of my favorite blocks of code is a 500 line method of 100% procedural code that controls camera behaviour for a game I worked on. The behaviour is extremely detailed and specific. I've modified the code there dozens of times. The ease by which I can add or modify features and the clarity that having it all as a single method provided challenged my assumptions about long methods (I used to be in your camp, I'm not anymore).

Bottom line is there are real benefits to keeping highly sequential, procedural code written in clear sequence and not broken up into pseudo hierarchy via method refactoring.

Again, I've outlined in detail my defense of single long methods, and when I believe they're appropriate. Without relying on normative statements, feelings, jargon, or 'code smell' - can you describe why you think they're a problem?

 

If you honestly have better results using a single large method, then you should go that route. What ever works best for you. In my experience, every time I have used a really long method, it has been more difficult to maintain that code over time. Breaking up large code into smaller methods always makes it easier for me to maintain code.

I don't let normative statements, feelings, jargon, or 'code smell' over influence my decisions. I have worked with enterprise code and game code, and I realize there is not one right answer. Each person has slightly different coding preferences, and games generally benefit from a slightly different coding style than enterprise apps. I personally strongly prefer multiple small methods instead of a single large method in both enterprise and game dev situations.

In the code example you just posted, I would personally break that method into at least two smaller methods.

 

There is value in discussion and disagreement, but only if both parties are willing to put forth reasoning, examples, etc. Solid, material, concrete points.

It's great that you've had decades of experience, and that you would refactor the above code. But why? Can you put together a concrete rationale for why refactoring that example is superior?

If we analyze the pros and cons of a specific example, perhaps we can both learn something. If not, then discussion won't go anywhere interesting.

 

I'll just add to this. . . another advantage of separating a long method into smaller chunks, is error reduction.

Let's say you use the same method in many places but there's an error or you want to add something to it this could be quite tedious to update and make sure you've updated it everywhere.

So instead of changing it many times you just have to update that smaller method / function and you're done.

Honestly, the only time I think large methods are worthwhile is when you're starting a new language. It just makes sense to have big methods and even dump everything all in one file as a proof of concept.

Once you get that you can start using different idioms.

For example, when I'm learning flutter there's no way I'm gonna understand state management from the get go. It is something I'll introduce after I have a working prototype.

 

To be honest your 3 points above are invalid. They are broken all the time when creating maintainable software. Take coroutines for examaple. They are basically mini workflow engines that break up the workflow (method) in smaller chunks. Example:

Code (CSharp):

1.     [CreateAssetMenu(menuName = "Tutorial/AttachAttachmentStep")]
2.     public class AttachAttachmentStep : TutorialStep
3.     {
4.         public RailSystemAttachment Prefab;
5.         public int RailIndex;
6.  
7.         public override IEnumerator Execute()
8.         {
9.             var firearm = Get<Firearm>();
10.             var railSystem = firearm.GetComponent<RailSystemContainer>().RailSystems[RailIndex];
11.             var attachment = Item.GetComponent<RailSystemAttachment>();
12.  
13.             yield return WaitForAttachmentGrab();
14.  
15.             yield return WaitForAttachmentHoveringOverRail(railSystem, attachment);
16.  
17.             yield return WaitForPlacementOnRail(railSystem, attachment);
18.         }
19.  
20.         private IEnumerator WaitForAttachmentGrab()
21.         {
22.             yield return Execute<InteractStep>(step =>
23.             {
24.                 step.ItemType = "Attachment";
25.                 step.Title = "Attach attachment";
26.                 step.Item = Item;
27.             });
28.         }
29.  
30.         private IEnumerator WaitForAttachmentHoveringOverRail(RailSystem.RailSystem railSystem, RailSystemAttachment attachment)
31.         {
32.  
33.             ShowPopup(railSystem.transform, "Hover over the rail with the attachment.");
34.  
35.             while (attachment.AttachedSystem != railSystem)
36.             {
37.                 if (attachment.IsCompletelyAttached)
38.                 {
39.                     yield return Execute<DetachAttachmentStep>(step => { step.Attachment = attachment; step.CorrectAttachmentPlacement = true; });
40.                 }
41.                 else
42.                     yield return null;
43.             }
44.         }
45.  
46.         private IEnumerator WaitForPlacementOnRail(RailSystem.RailSystem railSystem, RailSystemAttachment attachment)
47.         {
48.             ShowPopup(railSystem.transform, "Slide the attachment over the rail until you find a good position and let go.");
49.  
50.             while (!attachment.IsCompletelyAttached || attachment.AttachedSystem != railSystem)
51.                 yield return null;
52.         }
53.     }

 

When you're working with async / await the idea of reading code sequentially gets blown out the water - big style!

 

Not in c charp thanks to language supported features like Task and enumerators before that

 

Let's take a look at what this would look like as a single method.

Code (csharp):

1.  
2.     var firearm = Get< Firearm >();
3.     var railSystem = firearm.GetComponent< RailSystemContainer >().RailSystems[RailIndex];
4.     var attachment = Item.GetComponent< RailSystemAttachment >();
5.  
6.     yield return Execute< InteractStep >( step =>
7.     {
8.       step.ItemType = "Attachment";
9.       step.Title = "Attach attachment";
10.       step.Item = Item;
11.     } );
12.  
13.     while( true )
14.     {
15.       if( attachment.IsCompletelyAttached && attachment.AttachedSystem != railSystem )
16.         yield return Execute< DetachAttachmentStep >( step =>
17.         {
18.           step.Attachment = attachment;
19.           step.CorrectAttachmentPlacement = true;
20.         } );
21.       if( attachment.IsCompletelyAttached && attachment.AttachedSystem == railSystem )
22.         break;
23.     }
24.     ShowPopup( railSystem.transform, "Hover over the rail with the attachment." );
25.     while( attachment.AttachedSystem != railSystem )
26.     {
27.       if( attachment.IsCompletelyAttached )
28.         yield return Execute< DetachAttachmentStep >( step =>
29.         {
30.           step.Attachment = attachment;
31.           step.CorrectAttachmentPlacement = true;
32.         } );
33.       else
34.         yield return null;
35.     }
36.  
37.     ShowPopup( railSystem.transform, "Slide the attachment over the rail until you find a good position and let go." );
38.     while( !attachment.IsCompletelyAttached || attachment.AttachedSystem != railSystem )
39.       yield return null;
40.  

By looking at this all inline, it appears to me that this code is actually buggy. That if the user cancels the attachment process before it's completed, but puts it in the wrong slot (a condition covered in the second method) they aren't prompted with the detachment step again, since we're in the "Slide the attachment over the rail" step.

I could be mistaken, but I believe the code would actually be more correct if written as follows.

Code (csharp):

1.  
2. public override IEnumerator Execute()
3.   {
4.     var firearm = Get< Firearm >();
5.     var railSystem = firearm.GetComponent< RailSystemContainer >().RailSystems[RailIndex];
6.     var attachment = Item.GetComponent< RailSystemAttachment >();
7.  
8.     yield return Execute< InteractStep >( step =>
9.     { step.ItemType = "Attachment"; step.Title = "Attach attachment"; step.Item = Item; } );
10.  
11.     while( true )
12.     {
13.       yield return null;
14.       if( attachment.AttachedSystem != railSystem && attachment.IsCompletelyAttached )
15.       {
16.         yield return Execute< DetachAttachmentStep >( step =>
17.         {
18.           step.Attachment = attachment;
19.           step.CorrectAttachmentPlacement = true;
20.         } );
21.       }
22.       else if( attachment.AttachedSystem != railSystem && ! attachment.IsCompletelyAttached )
23.       {
24.         ShowPopup( railSystem.transform, "Slide the attachment over the rail until you find a good position and let go." );
25.       }
26.       else if( attachment.AttachedSystem == railSystem && ! attachment.IsCompletelyAttached)
27.       {
28.         ShowPopup( railSystem.transform, "Hover over the rail with the attachment." );
29.       }
30.       if( attachment.AttachedSystem == railSystem && attachment.IsCompletelyAttached )
31.         break;
32.     }
33.   }
34.  

Correct me if I'm wrong, but wouldn't the above code be more correct and less buggy?

If that fixed a bug in the sequencing (which I think it may have), then I think I made my case.

(EDIT: forgot the popups, so rejiggered it slightly to show correct popups. I would probably not organize the conditionals exactly like this as I think it's ugly, but whatever.)

 

Here is an example where I had the choice to create one 500 line method, or multiple smaller methods. I chose to split the code into smaller methods. I have class level variables that I do not pass between methods for performance reasons. The class is very performant, and the class has zero memory allocation per frame.

This is some code from my Instancing Pool class that I use for rendering many thousands of lasers without using a bunch of game objects. I originally wrote this for Unity 5.5, so this was before ECS was available to use for this type of solution.

Here is the Update method:

Code (CSharp):

1.     void Update()
2.     {
3.         CountActiveObjects();
4.  
5.         if (objectCount > 0)
6.         {
7.             MoveProjectiles();
8.  
9.             RenderProjectiles();
10.         }
11.     }

I could have inlined all of the code into that method and then used comments to break up that code, but I prefer it like this. It is easier for me to work with, including modifying and debugging.

 

Dude reading parallel code sequentially is an oxymoron. No coding language paradigm has solved this conundrum yet.
Async await makes it easier to digest than 'promises' though.

P.S I'm at stage 1 of 5 at conquering state management today

 

attachment.IsCompletelyAttached on the wrong rail will execute a DetachAttachmentStep step. When you have removed it it will return to previous state. Though it will not return all to beginning thats true. I reasoned like this, since he got the help to get that far he only need the help to detach it. And can return to the sliding part by his own. Will fix a framework so that you can define tutorial step sequences and return to the start if the player does something wrong.

Something like

Code (CSharp):

1. ExecuteSequence(WaitForAttachmentGrab, WaitForAttachmentHoveringOverRail, WaitForPlacementOnRail)

Then a custom yield instruction, TutorialStepResult with a states Waiting, StepCompleted, ReturnToStart or ReturnToPrevous step. Or similar


edit: If you think your rewrite is easier to read then we will never find a common ground
edit2: Your larger while loop can also be broken up to same methods I had btw.

 

Its not parallell, its asynchronous. though when dealing with Tasks it can be parallell, and it differs if you are on Full framework or Core Normally a developer does not need to worry about it though. Just treat the Task/coroutine as sequential. But sometimes you run into problems. Like I did here, I executed some code using Task.WhenAll which is parallell and ran into this problem

https://andersmalmgren.com/2019/08/29/a-proper-thread-safe-memory-cache/

 

Dude I don't use dotnet outside of unity for sanity reasons but I feel ya.

Yeah I guess people use the terms parallel, async, and multi-threaded interchangeably when they're very different.

My main exposure to async progging is through node and flutter. P.S nice blog.

 

Here is a possible way you could do to make your code example easier by splitting up the method until multiple smaller methods:

Code (CSharp):

1.  
2. if(DoesSettingExistForRequest())
3. {
4.     CreateRequestDTO();
5. }
6.  

 

The point is - that by actually having more of the working code all in one place - it can be more obvious that there are potential problems because you can read all the relevant logic in one pass.

By laying out the method in a single block, it became somewhat obvious that there were potential logical problems with the sequence.

I agree with you that the code itself is 'uglier' - but I'm personally less focused on the aesthetics of the code as I am on the functionality itself.

I prefer correct code over pretty code, and there are real advantages in clearly understanding the code when the full process can fit in a single sceen, all in sequence.

 

There is nothing wrong in the code though. It was a decision I took that the use case was fulfilled. So its not a bug caused by a miss, thats all. But I will fix mentioned framework tonight

 

Lol! I'm with you on the "its not a bug but I fix it tonight" thing. We've all been there!

 

If you scroll up to earlier in the thread, this is basically what Anders did, the full example of the refactored and original code is in this thread.

I am arguing that the original code is better than the refactored code and that splitting it up like you suggest does not objectively improve it.

 

haha, its not a bug. But I will improve it with a cool new framework, can not say No to make a new cool framework

 

New and improved version

The code that does the magic

Code (CSharp):

1.         protected IEnumerator Execute(params Func<IEnumerator>[] routines)
2.         {
3.             var sequence = new LinkedList<Func<IEnumerator>>(routines);
4.             var current = sequence.First;
5.  
6.             do
7.             {
8.                 var routine = current.Value();
9.                 var handPicked = false;
10.                 do
11.                 {
12.                     if (routine.Current is SequenceState state)
13.                     {
14.                         handPicked = true;
15.  
16.                         switch (state)
17.                         {
18.                             case SequenceState.Start:
19.                                 current = sequence.First;
20.                                 break;
21.                             case SequenceState.Previous:
22.                                 current = current.Previous ?? current;
23.                                 break;
24.                         }
25.  
26.                         break;
27.                     }
28.  
29.                     yield return routine.Current;
30.                 } while (routine.MoveNext());
31.  
32.                 if (!handPicked) current = current.Next;
33.             } while (current != null);
34.         }

Used like

Code (CSharp):

1.         public override IEnumerator Execute()
2.         {
3.             yield return Execute(WaitForAttachmentGrab, WaitForAttachmentHoveringOverRail, WaitForPlacementOnRail);
4.         }

Code (CSharp):

1.             ShowPopup(railSystem.transform, "Hover over the rail with the attachment.");
2.  
3.             while (attachment.AttachedSystem != railSystem)
4.             {
5.                 if (attachment.IsCompletelyAttached)
6.                 {
7.                     yield return Execute<DetachAttachmentStep>(step => { step.Attachment = attachment; step.CorrectAttachmentPlacement = true; });
8.                     yield return SequenceState.RestartCurrent;
9.                 }
10.                 else
11.                     yield return null;
12.             }

 

ok i think im half way there to understanding SM. useful day but i ain't gonna lie, I got a headache

Spoiler

 

IMO, now it is much much more complex. But at least it should work right...maybe

I still like just seeing the variables and stuff, cuz I can actually read all the code, but to each their own.

 

The first block is just framework code and can pretty much be how complex you want. Its the domain code that needs to be kept clean and simple.

 

@AndersMalmgren - thank you for being willing to show actual examples.

I may have different preferences for code, but its still interesting seeing how an experienced developer takes a different approach.

 

In the spirit of showing code, here's some of mine...

Code (CSharp):

1. void ExitGame()
2. {
3.    Application.Quit();
4. }

I hope you've all learned a valuable lesson from this.

 

Doing a abstraction like that even if it's private is fine. If it's just used from one place it might be overkill, but hey, if you add a second exit point its worth it right

 

It works just fine

Thanks for making me improve my code

edit: haha, it recorded my mic, ops

 

Remembered this thread today. Was trying to do method extraction on this code

Code (CSharp):

1.         protected async Task PollQueue(CancellationToken cancellationToken)
2.         {
3.             var queue = new Queue<Batch>();
4.             var runners = new Dictionary<Guid, Task<Batch>>();
5.             var lastCheck = DateTime.MinValue;
6.          
7.             do
8.             {
9.                 if (!cancellationToken.IsCancellationRequested)
10.                 {
11.  
12.                     var slots = Environment.ProcessorCount - runners.Count;
13.                     if (queue.Count < slots && DateTime.Now - lastCheck > TimeSpan.FromSeconds(1))
14.                     {
15.                         using (var scope = _serviceProvider.CreateScope())
16.                         {
17.                             var batch = (await scope.ServiceProvider.GetService<ICommandRepository>()
18.                                     .ListByAsync(CommandState.Pending))
19.                                     .Where(c => !runners.ContainsKey(c.BatchId))
20.                                     .GroupBy(c => c.BatchId)
21.                                     .Select(grp => new Batch {Id = grp.Key, Commands = grp.ToList()});
22.  
23.                             queue = new Queue<Batch>(batch);
24.                             if (queue.Count == 0)
25.                                 lastCheck = DateTime.Now;
26.                         }
27.                     }
28.  
29.                     var tempQueue = queue;
30.                     Enumerable
31.                         .Range(0, Math.Min(slots, queue.Count))
32.                         .ForEach(i =>
33.                         {
34.                             var batch = tempQueue.Dequeue();
35.                             var runner = Task.Run(async () =>
36.                             {
37.                                 await batch
38.                                     .Commands
39.                                     .Select(ExecuteCommand)
40.                                     .WaitAll(cancellationToken);
41.  
42.                                 return batch;
43.                             });
44.                             runners[batch.Id] = runner;
45.                         });
46.                 }
47.  
48.                 if (runners.Count == 0) break;
49.  
50.                 var completed = await Task.WhenAny(runners.Values);
51.                 runners.Remove(completed.Result.Id);
52.  
53.             } while (true);
54.         }

Its one of those rare times when method extraction is impossible or make the code harder to read. It has alot of state that needs to be shared between the 3 sections of code. Ideally it would be broken up into RefreshQueue, ExecuteFromQueue and WaitForRunner or similar names

 

I personally prefer Nito's AsyncReaderWriterLock for async locking over async semaphores. I assume you left off abit of the boilerplate code that should go around such a dangerous operation, so I won't comment on it =P.

If the cache isn't threadsafe then maybe it should handle concurrency and locking internally, and not calling code. It would be ideal if reading from the cache only acquired a read-level lock too. Since it seems multiple threads may be interacting with it otherwise the blocking locks will hurt throughput.

NUnit and either NCrunch or newer VS Test Explorer.

 

I think Microsoft choose to make the Memory cache threadafe but left this part out so it's up to the caller if it's OK for the factory to potentially be called multiple times.