Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
B-Nasty
May 25, 2005

The Wizard of Poz posted:

I should mention though that I've had to teach myself mostly by reading millions of articles across the web, so this might not be the bestest possible way of structuring things, but I've found it works well for our needs.

That's generally how I structure it. The controller methods should be a few lines long, typically.

In some cases, a CQRS approach works well here, though that can get messy/overkill for many applications. I've also used AutoMapper for some of the boring Model<->DTO stuff, but again, sometimes it gets in the way more than it helps.

Adbot
ADBOT LOVES YOU

putin is a cunt
Apr 5, 2007

BOY DO I SURE ENJOY TRASH. THERE'S NOTHING MORE I LOVE THAN TO SIT DOWN IN FRONT OF THE BIG SCREEN AND EAT A BIIIIG STEAMY BOWL OF SHIT. WARNER BROS CAN COME OVER TO MY HOUSE AND ASSFUCK MY MOM WHILE I WATCH AND I WOULD CERTIFY IT FRESH, NO QUESTION

B-Nasty posted:

In some cases, a CQRS approach works well here, though that can get messy/overkill for many applications. I've also used AutoMapper for some of the boring Model<->DTO stuff, but again, sometimes it gets in the way more than it helps.

Yeah I use AutoMapper quite a bit for that conversion stuff, but opted to exclude it from the example for brevity. It's a good little tool, I wouldn't recommend anyone use the reflection-based features too extensively though, they're expensive as gently caress.

EssOEss
Oct 23, 2006
128-bit approved
I have recently taken up using AutoMapper and I am cautiously optimistic but I sense that I do not really have a feel for it. Would you care to share some experience and showcase what you consider best practices?

LOOK I AM A TURTLE
May 22, 2003

"I'm actually a tortoise."
Grimey Drawer

EssOEss posted:

I have recently taken up using AutoMapper and I am cautiously optimistic but I sense that I do not really have a feel for it. Would you care to share some experience and showcase what you consider best practices?

Some suggestions based on my experiences with AutoMapper:
- Avoid expensive operations such as data store queries in mapping expressions. You'll typically be running them synchronously on the main thread.
- Only map from domain/data store objects to DTOs/POCOs, never the other way around. The creator of AutoMapper himself has talked about this on multiple occasions: https://lostechies.com/jimmybogard/2009/09/18/the-case-for-two-way-mapping-in-automapper/
- Map types as late as possible. Typically I like to call Map right before returning to the caller.

I'm currently working on an ASP.NET Core API using Entity Framework and AutoMapper. A pattern I'm using extensively is to use the Include method to eagerly load related entities and then use AutoMapper to transform or flatten the entities into DTOs. Imagine a DB model like this:

C# code:
public class Product
{
	public int Id { get; set; }
	public int TypeId { get; set; }
	public ProductType Type { get; set; }
}

public class ProductType
{
	public int Id { get; set; }
	public string Name { get; set; }
	public ICollection<Product> Products { get; set; }
}
Perhaps my third party clients shouldn't have to care that I have a ProductType entity in my model, so when I expose the Product model I flatten the type name value from the parent table into a single DTO:
C# code:
public class ProductDto
{
	public int Id { get; set; }
	public string Type { get; set; }
}
My mapping definition would look something like this:
C# code:
public class MappingProfile : Profile
{
	public MappingProfile()
	{
		// I would let AutoMapper handle the Id field automatically since it has the name/type both on the source and the destination.
		// Some people like to be more explicit, which is also fine.
		CreateMap<Product, ProductDto>()
			.ForMember(dest => dest.Type, options => options.MapFrom(source => source.Type.Name));
	}
}
If I have a controller method GetProductById I might implement it something like this (some details omitted obviously):
C# code:
public class ProductController : Controller
{
	private readonly MyDbContext _context;
	private readonly IMapper _mapper;

	public ProductController(MyDbContext context, IMapper mapper) // assuming use of DI to get these values
	{
		_context = context;
		_mapper = mapper;
	}

	[HttpGet("{id}")]
	public async Task<IActionResult> GetProductById([FromRoute] int id)
	{
		// some sanity checks omitted, of course
		var product = await _context.Products
			.Include(p => p.Type)
			.SingleOrDefaultAsync(p => p.Id == id);
		var mapped = _mapper.Map<ProductDto>(product);
		return Whatever(mapped);
	}
}
Note that AutoMapper will automatically ignore NullReferenceExceptions in your mapping expressions, so if you didn't load the related ProductType entity before calling Map the expression p => p.Type.Name in the mapping configuration will just result in the Name field being null. Some people find this distasteful, but I think it's okay as long as you don't put anything too complex in your mapping expressions.

A good safety measure is to create a unit test that calls the Mapper.AssertConfigurationIsValid method. This will catch certain mistakes, such as if you're missing a mapping expression for some property on your target type. Typically the test will look like this:

C# code:
public class MappingProfileTest
{
	[Fact]
	public void MappingIsValid()
	{
		Mapper.Initialize(config => config.AddProfile<MappingProfile>()); // this removes any other mappings in the global Mapper object
		Mapper.AssertConfigurationIsValid();
	}
}
TL;DR: Use AutoMapper. It's :thumbsup:

epswing
Nov 4, 2003

Soiled Meat

LOOK I AM A TURTLE posted:

- Map types as late as possible. Typically I like to call Map right before returning to the caller.

How come?

Opulent Ceremony
Feb 22, 2012
The main reason to use AutoMapper is for ProjectTo with EF, which lets you select specific columns from a table without having to write the the same Select(x => new {}) in multiple places.

ModeSix
Mar 14, 2009

Furism posted:

Now that you can get SQL Server for Linux I might just switch to that honestly. Is there any downside to use SQL Server vs MariaDB/PostgreSQL if you don't care whether or not the code is open source?

I think the fact that the it says it requires 3.5GB of RAM is kind of a reason to use another dB provider on Linux.

3.5gb seems excessive.

Iverron
May 13, 2012

B-Nasty posted:

That's generally how I structure it. The controller methods should be a few lines long, typically.

In some cases, a CQRS approach works well here, though that can get messy/overkill for many applications. I've also used AutoMapper for some of the boring Model<->DTO stuff, but again, sometimes it gets in the way more than it helps.

My controllers all take a single dependency: IMediator. They either take Queries on GET or Commands on POST (99% of the time) and send them through the mediator to a handler that lives in the same file as the Query / Command.

Most actions look something like this:

code:
[Route("list/{Butt}/{Fart}")]
public ActionResult List(List.Query query)
{
	var smells = _mediator.Send(query);
	return View(smells);
}
The List object looks something like:

code:
public class List
{
        public class Query : IRequest<IEnumerable<Result>> 
	{
		public string Butt { get; set; }
		public string Fart { get; set; }
	}

        public class Result
        {
            	public string Smell { get; set; }
		public int Rating { get; set; }
        } 

        public class Handler : IRequestHandler<Query, IEnumerable<Result>> 
        {
           	private readonly SqlContext _context;
            	public Handler(SqlContext context)
            	{
                	_context = context;
            	}

            	public IEnumerable<Result> Handle(Query message)
            	{
                	var results = new List<Result>();
                
			//magic

                	return results;
            	}
        }
}
Super simple. None of the 4 line long controller dependencies. Heavily influenced by Jimmy Bogard's posts on using Mediatr.

If things get more complex, I start looking into introducing DDD or separating into layers.

Iverron fucked around with this message at 22:19 on Dec 21, 2016

jony neuemonic
Nov 13, 2009

Iverron posted:

My controllers all take a single dependency: IMediator. They either take Queries on GET or Commands on POST (99% of the time) and send them through the mediator to a handler that lives in the same file as the Query / Command.

Most actions look something like this:

code:

[Route("list/{Butt}/{Fart}")]
public ActionResult List(List.Query query)
{
	var smells = _mediator.Send(query);
	return View(smells);
}

The List object looks something like:

code:

public class List
{
        public class Query : IRequest<IEnumerable<Result>> 
	{
		public string Butt { get; set; }
		public string Fart { get; set; }
	}

        public class Result
        {
            	public string Smell { get; set; }
		public int Rating { get; set; }
        } 

        public class Handler : IRequestHandler<Query, IEnumerable<Result>> 
        {
           	private readonly SqlContext _context;
            	public Handler(SqlContext context)
            	{
                	_context = context;
            	}

            	public IEnumerable<Result> Handle(Query message)
            	{
                	var results = new List<Result>();
                
			//magic

                	return results;
            	}
        }
}

Super simple. None of the 4 line long controller dependencies. Heavily influenced by Jimmy Bogard's posts on using Mediatr.

If things get more complex, I start looking into introducing DDD or separating into layers.

This is exactly how I've been structuring things lately (right down to the query/handler/result in one file) and it's been working great.

EssOEss
Oct 23, 2006
128-bit approved

LOOK I AM A TURTLE posted:

- Only map from domain/data store objects to DTOs/POCOs, never the other way around. The creator of AutoMapper himself has talked about this on multiple occasions: https://lostechies.com/jimmybogard/2009/09/18/the-case-for-two-way-mapping-in-automapper/

The article seems to just say "Why would you want to?" without really explaining. I do this and I would like to better understand why this is not proper.

In my case, I have say a GET and POST operation in a web API. They communicate using data structures that are part of the API contract. POST uses AutoMapper to later transform that to a business logic object, pushing it through my business logic classes, which in turn may eventually shovel it over to a persistence component that transforms it internally to some DTO for storage.

My GET operation does the same chain in reverse - the storage system uses AutoMapper to create business logic objects from the formatted-for-storage objects, which pass through whatever business logic may be needed (not much for GET, but you know - simplistic example) and the API call handler finally transforms it using AutoMapper to the data structure defined in the API contract.

In this sense, it's a 3 layer architecture, consisting of web API, business logic and storage. Each has its own data structures and transformations of some sort occur to go from one to the next and back.

Why is this bad?

LOOK I AM A TURTLE
May 22, 2003

"I'm actually a tortoise."
Grimey Drawer

epalm posted:

How come?

Just my personal preference. In the case of mapping domain objects to DTOs, performing the mapping right before returning ensures that your controller classes don't really need to care about the structure of the DTOs at all. I think the main strength of AutoMapper is that you can change the behavior of the mapping in multiple API methods at the same time by changing the mapping definition, but if you're doing manual handling of the mapped object (EDIT: I forgot to finish this sentence. I meant to say that if you do manual handling after mapping you lose the strength of defining the mapping in a single place). Of course, in practice this is a "rule" you'll have to break on a pretty regular basis.

EssOEss posted:

The article seems to just say "Why would you want to?" without really explaining. I do this and I would like to better understand why this is not proper.

In my case, I have say a GET and POST operation in a web API. They communicate using data structures that are part of the API contract. POST uses AutoMapper to later transform that to a business logic object, pushing it through my business logic classes, which in turn may eventually shovel it over to a persistence component that transforms it internally to some DTO for storage.

My GET operation does the same chain in reverse - the storage system uses AutoMapper to create business logic objects from the formatted-for-storage objects, which pass through whatever business logic may be needed (not much for GET, but you know - simplistic example) and the API call handler finally transforms it using AutoMapper to the data structure defined in the API contract.

In this sense, it's a 3 layer architecture, consisting of web API, business logic and storage. Each has its own data structures and transformations of some sort occur to go from one to the next and back.

Why is this bad?

First off, I shouldn't have said "never". Rules in software development are meant to be broken, and on the surface this seems like a fairly arbitrary rule.

To be a little more specific, it probably depends on how your classes are structured. If all your classes are basically 1-to-1 mappings of each other and only exist as separate classes for architectural purposes then it's probably fine to configure and use two-way mappings. However in many cases the domain model object is likely to be more complex than the models you're exposing to clients, in which case the mapping from DTO to domain is also going to be complex or even impossible.

In my experience, bugs in mapping definitions are typically harder to track down than bugs in manual mapping code. An easy trap to fall into with AutoMapper is to create a bunch of mapping definitions that do too much and are used exactly once in your code, at which point you're just needlessly complicating your code and debugging experience, and probably also taking the ability to find logical errors away from the compiler and static analysis tools. After all, a call to Map<T>() will never fail at compile time.

If your domain and/or storage classes are as simple as your DTOs, or if the mappings are sufficiently simple that they don't cause you problems, go right ahead. I imagine this would be typical for a system with a fairly thin business logic layer, where the API exposed to the client is a quite faithful representation of the underlying domain model.

Some other SO posts on this subject:
http://stackoverflow.com/questions/39535559/mapping-converting-data-between-the-data-layer-and-the-logic-layer
http://stackoverflow.com/questions/14229166/domain-object-to-viewmodel-vice-versa-using-automapper
http://stackoverflow.com/questions/8486756/automapper-from-view-to-domain/8490183#8490183

LOOK I AM A TURTLE fucked around with this message at 12:41 on Dec 22, 2016

putin is a cunt
Apr 5, 2007

BOY DO I SURE ENJOY TRASH. THERE'S NOTHING MORE I LOVE THAN TO SIT DOWN IN FRONT OF THE BIG SCREEN AND EAT A BIIIIG STEAMY BOWL OF SHIT. WARNER BROS CAN COME OVER TO MY HOUSE AND ASSFUCK MY MOM WHILE I WATCH AND I WOULD CERTIFY IT FRESH, NO QUESTION

EssOEss posted:

In my case, I have say a GET and POST operation in a web API. They communicate using data structures that are part of the API contract. POST uses AutoMapper to later transform that to a business logic object, pushing it through my business logic classes, which in turn may eventually shovel it over to a persistence component that transforms it internally to some DTO for storage.

Having an API model that is complex enough to build up a complete business logic object and persistence object is pretty rare.

What about things like a UserId, the user who created the object, is that in your API object?

Furism
Feb 21, 2006

Live long and headbang

ModeSix posted:

I think the fact that the it says it requires 3.5GB of RAM is kind of a reason to use another dB provider on Linux.

3.5gb seems excessive.

I figure MS SQL is quite a different beast than MySQL or pgSQL and is more similar to Oracle, feature-wise, so we can't really compare the hardware requirements. But at the same time I don't need something like Oracle so you're probably right.

thebigcow
Jan 3, 2001

Bully!
Does it actually need that, or is that the suggested minimum system requirement for whatever distro they tested with.

EssOEss
Oct 23, 2006
128-bit approved

The Wizard of Poz posted:

What about things like a UserId, the user who created the object, is that in your API object?

No, of course not. Hmm, I think I understand the reasoning now - there seems to be an underlying assumption that the business logic object is some fat feature-rich blob. This is not the case in my software - the business logic is just another API and the objects one works with are just the input to that API. There is no UserId of the creator in the business logic objects created from the API layer objects, either, as that is part of the business logic domain itself and not necessarily part of the API (at least the part that supplies input to the business logic - it can of course output the creator's UserId).

Rocko Bonaparte
Mar 12, 2002

Every day is Friday!
I have a json.net question. I'm trying to figure out why I get an ArgumentException trying to do this:
code:
    public void Load(string json)
    {
        var root = JsonConvert.DeserializeObject<Dictionary<string, JObject>>(json);
        
        var headerMap = root["Header"].ToObject<Dictionary<string, JObject>>();   // DEAD RIGHT HERE
        ...
    }
on this:

code:
{
  "Header": {
      "SimpleStringHeader": "Can I load this?"
   },
  "Objects: {}
}
Let's grant that this is kind of stupid to do, but I'm oversimplifying. The data I'm dealing with tends to have a lot of stuff, and the data types are usually compound. However, this one just needs to spit out one string and that's it.

Like, this input would work fine:
code:
{
  "Header": {
      "SimpleStringHeader": {   // Wrapping the string in a compound object
        "Value": "Can I load this?"
      }
   },
  "Objects: {}
}
I just don't want to write a boilerplate class just for a string.

I was figuring a string was a JObject too, but I guess not.

Rocko Bonaparte fucked around with this message at 09:24 on Jan 2, 2017

rarbatrol
Apr 17, 2011

Hurt//maim//kill.
JObject inherits from JToken, which I think is what strings are deserialized as. You can probably deserialize as a Dictionary<string, JToken> without problems.

Destroyenator
Dec 27, 2004

Don't ask me lady, I live in beer
You can also do it anonymous types if you don't want to write a class.
code:
var thing = JsonConvert.DeserializeAnonymousType(json, new { Header = new { SimpleStringHeader = "" }});
var simple = thing.Header.SimpleStringHeader;

Rocko Bonaparte
Mar 12, 2002

Every day is Friday!

rarbatrol posted:

JObject inherits from JToken, which I think is what strings are deserialized as. You can probably deserialize as a Dictionary<string, JToken> without problems.

Heh, it all casted pretty easily to JToken, but it looks like that wasn't it. When I actually passed in a string, ToString() would strip the quotes and gently caress up DeserializeObject. After a lot of fruitless googling, I had an epiphany to, you know, not bother with DeserializeObject if I already have a string. Specifically, later in the code:

code:
        Type recordType = serializerMap[serializerName].GetRecordType();

        JToken token = thisObjectSerializers[serializerName];
        object serializedData;
        if (token.Type == JTokenType.String)
        {
            serializedData = token.ToString();
        }
        else
        {
            serializedData = JsonConvert.DeserializeObject(token.ToString(), recordType);
        }

Destroyenator posted:

You can also do it anonymous types if you don't want to write a class.
code:
var thing = JsonConvert.DeserializeAnonymousType(json, new { Header = new { SimpleStringHeader = "" }});
var simple = thing.Header.SimpleStringHeader;
The real code gets a little too complicated for this. I'm trying to create a specific save system for a game I'm doodling on in Unity. I'm scanning for everything that implements my saving type and then having them drop some data into my lookup. They're all sorted by each object and then what type of data they're using. I guess it's coming down to something like a Momento pattern. I'm not too keen on it because of all the boilerplate I'm adding to do this. Anonymous types were something I pondered for that, but I couldn't see putting it all together. The boilerplate right now is:

1. Creating the transformation class that takes the existing component in-game and farms off the data that is necessary to save. It also manages loading the data back in to a component.
2. Creating a basic class with name-value mappings.

If I just tried to serialize the components that Unity uses, I get a ton of confusing poo poo and state that I don't necessarily even want to carry over. I was hoping I could maybe reduce that basic class to an anonymous because it gets quite tedious, but I can't see how to do that and refer to the class in two different places; I figured once I needed it in more than one place, it was a class.

I've decided to live with it because having the name-value mapping class lets me decide how to name stuff for export, which does give me some more flexibility. It's just that a lot of the time, I don't even use it.

Portland Sucks
Dec 21, 2004
༼ つ ◕_◕ ༽つ
I'm working on a WPF desktop application that needs to be deployed to a Surface Pro where the user is going to be walking around with this thing in and out of Wi-Fi coverage. The primary function of the program is a real time monitor of data so it needs to be both polling the database regularly in order to keep the home screen refreshed but also needs to be functional when the user steps out of wireless coverage temporarily. I've been trying to make use of async/await pretty heavily in the development of this but can't get away from the program hanging as soon as I walk outside of wireless coverage. It seems to work fine otherwise, and even works smoothly if I manually disconnected from the wireless network all together and then reestablish a connection.

My process has been to spool up the async loop that will handle the continuous refreshing like so...

code:
        private static async void RunPeriodicAsync(Action onTick, TimeSpan dueTime, TimeSpan interval, CancellationToken token)
        {
            if (dueTime > TimeSpan.Zero)
                await Task.Delay(dueTime, token);

            while (!token.IsCancellationRequested)
            {
                onTick.Invoke();

                if (interval > TimeSpan.Zero)
                    await Task.Delay(interval, token);
            }

        }
and on each iteration check if I can establish a valid connection to the database, and if I can then go ahead and do the database stuff

code:
        private void OnTick()
        {
            if ( SQLQuery.IsServerConnected() )
            {
                System.Console.WriteLine("Server connection good. Refreshing...");
                //QueryDataBaseAndDoSomeStuff();
                
            }

        }
when I check the database connection all i'm doing is the following

code:

        public static bool IsServerConnected()
        {
             
            ## connection string omitted ##

            using (var dbConnection = new SqlConnection(builder.ConnectionString))
            {
                try
                {

                    dbConnection.OpenAsync();
                    return true;
                }
                catch (SqlException)
                {
                    return false;
                }
            }
        }

This all works well and good as long as I stay within wireless connectivity, but as soon as I walk outside of a good connection the program hangs here
code:
if ( SQLQuery.IsServerConnected() )
. I feel like this shouldn't be happening since the connection verification step is happening asynchronously from the UI thread and it doesn't hang if I just straight up turn off the WiFi connection by hand. Something about being in that intermediate "bad signal" area seems to be causing a deadlock or something.

Has anyone dealt with anything similar or is there something I'm doing that is glaringly wrong?

raminasi
Jan 25, 2005

a last drink with no ice
Here's a fun one: some kind of bug within a function is so disastrous that it is propagating itself back in time to prevent the function itself from ever being called, or something:

C# code:
public void Whatever()
{
    Console.WriteLine("butts");
    SomethingComplicated();
    Console.WriteLine("farts");
}
Unless the second line is commented out, this doesn't print anything, and a breakpoint on the first line is never hit. What the hell is going on?

e: I figured it out, it was a MissingMethodException somewhere caused by an assembly version mismatch, but goddamn that is infuriating.

raminasi fucked around with this message at 22:23 on Jan 3, 2017

Bognar
Aug 4, 2011

I am the queen of France
Hot Rope Guy
Are you seeing any exceptions when you execute the method?

Here's the only thing I can think of: the CLR JITter tries to compile a function in full before it executes it, and while doing that it will attempt to resolve any types that are being used by the method. That could involve running a static constructor on types that have not yet been resolved. If that static constructor throws (or, say, overflows the stack), then the initial method that is being compiled may never execute.

e: oh yep, that'll do it

gariig
Dec 31, 2004
Beaten into submission by my fiance
Pillbug

Portland Sucks posted:

I'm working on a WPF desktop application that needs to be deployed to a Surface Pro where the user is going to be walking around with this thing in and out of Wi-Fi coverage. The primary function of the program is a real time monitor of data so it needs to be both polling the database regularly in order to keep the home screen refreshed but also needs to be functional when the user steps out of wireless coverage temporarily. I've been trying to make use of async/await pretty heavily in the development of this but can't get away from the program hanging as soon as I walk outside of wireless coverage. It seems to work fine otherwise, and even works smoothly if I manually disconnected from the wireless network all together and then reestablish a connection.

My process has been to spool up the async loop that will handle the continuous refreshing like so...

This all works well and good as long as I stay within wireless connectivity, but as soon as I walk outside of a good connection the program hangs here
code:
if ( SQLQuery.IsServerConnected() )
. I feel like this shouldn't be happening since the connection verification step is happening asynchronously from the UI thread and it doesn't hang if I just straight up turn off the WiFi connection by hand. Something about being in that intermediate "bad signal" area seems to be causing a deadlock or something.

Has anyone dealt with anything similar or is there something I'm doing that is glaringly wrong?

Most likely the await for RunPeriodicAsync is capturing the WPF SynchronizationContext which runs on the UI thread, which is the default (blog post). If you return a Task and set ConfigureAwait(false) that should fix the issue. Another would be to spawn a Task to run onTick.Invoke() and a Task.Delay and use Task.WhenAny on the two tasks. You can check the TaskStatus of the onTick.Invoke Task to see if it completed. Here's a SO question about how to do this

Doing this might cause issues of QueryDataBaseAndDoSomeStuff(); is touching the UI and will need to be marshaled to the UI thread. Another option would be to split the pieces apart. One goes to the database to retrieve data and stick into a Concurrent Queue. Another on the UI thread reads the queue and updates the UI.

VVVV Also yeah since you are marshalled to the UI thread calling a synchronous blocking method stops the UI thread

gariig fucked around with this message at 22:35 on Jan 3, 2017

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

Portland Sucks posted:

I'm working on a WPF desktop application that needs to be deployed to a Surface Pro where the user is going to be walking around with this thing in and out of Wi-Fi coverage. The primary function of the program is a real time monitor of data so it needs to be both polling the database regularly in order to keep the home screen refreshed but also needs to be functional when the user steps out of wireless coverage temporarily. I've been trying to make use of async/await pretty heavily in the development of this but can't get away from the program hanging as soon as I walk outside of wireless coverage. It seems to work fine otherwise, and even works smoothly if I manually disconnected from the wireless network all together and then reestablish a connection.

My process has been to spool up the async loop that will handle the continuous refreshing like so...

code:
        private static async void RunPeriodicAsync(Action onTick, TimeSpan dueTime, TimeSpan interval, CancellationToken token)
        {
            if (dueTime > TimeSpan.Zero)
                await Task.Delay(dueTime, token);

            while (!token.IsCancellationRequested)
            {
                onTick.Invoke();

                if (interval > TimeSpan.Zero)
                    await Task.Delay(interval, token);
            }

        }
and on each iteration check if I can establish a valid connection to the database, and if I can then go ahead and do the database stuff

code:
        private void OnTick()
        {
            if ( SQLQuery.IsServerConnected() )
            {
                System.Console.WriteLine("Server connection good. Refreshing...");
                //QueryDataBaseAndDoSomeStuff();
                
            }

        }
when I check the database connection all i'm doing is the following

code:

        public static bool IsServerConnected()
        {
             
            ## connection string omitted ##

            using (var dbConnection = new SqlConnection(builder.ConnectionString))
            {
                try
                {

                    dbConnection.OpenAsync();
                    return true;
                }
                catch (SqlException)
                {
                    return false;
                }
            }
        }

This all works well and good as long as I stay within wireless connectivity, but as soon as I walk outside of a good connection the program hangs here
code:
if ( SQLQuery.IsServerConnected() )
. I feel like this shouldn't be happening since the connection verification step is happening asynchronously from the UI thread and it doesn't hang if I just straight up turn off the WiFi connection by hand. Something about being in that intermediate "bad signal" area seems to be causing a deadlock or something.

Has anyone dealt with anything similar or is there something I'm doing that is glaringly wrong?

The second you call a synchronous method, your async method will behave synchronously.

public static bool IsServerConnected should be public static async Task<bool> IsServerConnectedAsync and should be awaited. In the method itself, await the call to OpenAsync. OnTick should be async Task unless it's an event handler, in which case it should be async void.

Bognar
Aug 4, 2011

I am the queen of France
Hot Rope Guy

Portland Sucks posted:

Has anyone dealt with anything similar or is there something I'm doing that is glaringly wrong?

When you lose connectivity but your OS hasn't accepted it yet, database interactions may not immediately fail and could instead run into the connection timeout before failing. You could try setting your timeout lower. However, your bigger problem seems to be handling asynchrony incorrectly.

I also notice that your IsServerConnected method is calling OpenAsync inside a non-async function and the task is never waited on. From what you've posted, it looks like IsServerConnected will always return true.

Bognar fucked around with this message at 22:38 on Jan 3, 2017

Bognar
Aug 4, 2011

I am the queen of France
Hot Rope Guy
I'm just gonna take a stab and assume that you don't have a good handle on async/await, so I'll refer you to a post I made earlier in the thread:

https://forums.somethingawful.com/showthread.php?threadid=3644791&pagenumber=154&perpage=40#post466664662

Portland Sucks
Dec 21, 2004
༼ つ ◕_◕ ༽つ

New Yorp New Yorp posted:

The second you call a synchronous method, your async method will behave synchronously.

public static bool IsServerConnected should be public static async Task<bool> IsServerConnectedAsync and should be awaited. In the method itself, await the call to OpenAsync. OnTick should be async Task unless it's an event handler, in which case it should be async void.

Oh snap, somehow missed that nuance. Thanks for the heads up.

gariig posted:

Most likely the await for RunPeriodicAsync is capturing the WPF SynchronizationContext which runs on the UI thread, which is the default (blog post). If you return a Task and set ConfigureAwait(false) that should fix the issue. Another would be to spawn a Task to run onTick.Invoke() and a Task.Delay and use Task.WhenAny on the two tasks. You can check the TaskStatus of the onTick.Invoke Task to see if it completed. Here's a SO question about how to do this

Doing this might cause issues of QueryDataBaseAndDoSomeStuff(); is touching the UI and will need to be marshaled to the UI thread. Another option would be to split the pieces apart. One goes to the database to retrieve data and stick into a Concurrent Queue. Another on the UI thread reads the queue and updates the UI.

VVVV Also yeah since you are marshalled to the UI thread calling a synchronous blocking method stops the UI thread

Thanks for the links. I've been working through the Concurrency in C# Cookbook and reading the author's blog but it seems like all of these little details are just spread out all over the place.

Portland Sucks
Dec 21, 2004
༼ つ ◕_◕ ༽つ

Bognar posted:

I'm just gonna take a stab and assume that you don't have a good handle on async/await, so I'll refer you to a post I made earlier in the thread:

https://forums.somethingawful.com/showthread.php?threadid=3644791&pagenumber=154&perpage=40#post466664662

Yeah, I'm using the Concurrency in C# Cookbook and trying to wrangle the MSDN documentation. I've done a little bit of basic multithreading in the past, but nothing quite like this before.

Portland Sucks
Dec 21, 2004
༼ つ ◕_◕ ༽つ

Bognar posted:

When you lose connectivity but your OS hasn't accepted it yet, database interactions may not immediately fail and could instead run into the connection timeout before failing. You could try setting your timeout lower. However, your bigger problem seems to be handling asynchrony incorrectly.

I also notice that your IsServerConnected method is calling OpenAsync inside a non-async function and the task is never waited on. From what you've posted, it looks like IsServerConnected will always return true.

Could you help me understand this strange output...I went ahead with your advice so I'm looking like this now

code:
        private static async void RunPeriodicAsync(Action onTick, TimeSpan dueTime, TimeSpan interval, CancellationToken token)
        {
            if (dueTime > TimeSpan.Zero)
                await Task.Delay(dueTime, token);

            while (!token.IsCancellationRequested)
            {
                onTick.Invoke();

                if (interval > TimeSpan.Zero)
                    await Task.Delay(interval, token);
            }

        }

code:

        private async void OnTick()
        {
            bool result = await SQLQuery.IsServerConnectedAsync();

            if (result)
            {
                ConnectedText = "Connected";
                System.Console.WriteLine("Result is : " + result.ToString() + "  Server connection good. Refreshing...");
            }
            else if (!result)
                System.Console.WriteLine("Result is : " + result.ToString() + "  Cannot connect to database. Attempting to reconnect...");
                ConnectedText = "Error";

        }

code:

        public static async Task<bool> IsServerConnectedAsync()
        {
            
            ...string builder stuff
            builder["Connection Timeout"] = "1";

            using (var dbConnection = new SqlConnection(builder.ConnectionString))
            {
                try
                {
                    await dbConnection.OpenAsync();
                    return true;
                }
                catch (SqlException)
                {
                    return false;
                }
            }
        }

ConnectedText is a public property that a text label in the UI has a data binding to. Right now the label is reading "Error", but the console output is Server Connected...

If I pull the network connection now it keeps chugging along for another 10-20 seconds and then starts throwing the SQLExceptions while the program hangs until I reestablish the connection.

The bound text never switches to Connected at any point which seems really odd to me.

New Yorp New Yorp
Jul 18, 2003

Only in Kenya.
Pillbug

Portland Sucks posted:

Could you help me understand this strange output...I went ahead with your advice so I'm looking like this now

ConnectedText is a public property that a text label in the UI has a data binding to. Right now the label is reading "Error", but the console output is Server Connected...

If I pull the network connection now it keeps chugging along for another 10-20 seconds and then starts throwing the SQLExceptions while the program hangs until I reestablish the connection.

The bound text never switches to Connected at any point which seems really odd to me.

You just learned a lesson:

ALWAYS use curly braces. Your indentation is misleading you:

code:
            if (result)
            {
                ConnectedText = "Connected";
                System.Console.WriteLine("Result is : " + result.ToString() + "  Server connection good. Refreshing...");
            }
            else if (!result)
                System.Console.WriteLine("Result is : " + result.ToString() + "  Cannot connect to database. Attempting to reconnect...");
                ConnectedText = "Error";
This is actually equivalent to:
code:
            if (result)
            {
                ConnectedText = "Connected";
                System.Console.WriteLine("Result is : " + result.ToString() + "  Server connection good. Refreshing...");
            }
            else if (!result)
            {
                System.Console.WriteLine("Result is : " + result.ToString() + "  Cannot connect to database. Attempting to reconnect...");
            }
            ConnectedText = "Error";
Use curly braces. Even for one liners. Just do it. Also, why not just write it like this?
code:
            if (result)
            {
                ConnectedText = "Connected";
                System.Console.WriteLine($"Result is {result}. Server connection good. Refreshing...");
            }
            else
            {
                System.Console.WriteLine($"Result is {result}. Cannot connect to database. Attempting to reconnect...");
                ConnectedText = "Error";
            }
Assuming you're using C# 6, $ is for string interpolation and makes writing things like that a lot cleaner. If you're not using C# 6, I'd still prefer using string.Format over string concatenation. String concatenation is ugly.

New Yorp New Yorp fucked around with this message at 23:52 on Jan 3, 2017

Portland Sucks
Dec 21, 2004
༼ つ ◕_◕ ༽つ

New Yorp New Yorp posted:

You just learned a lesson:

ALWAYS use curly braces. Your indentation is misleading you:


Oh god. Sorry, I'm a python developer venturing into a brave new world. Old habits die hard.

putin is a cunt
Apr 5, 2007

BOY DO I SURE ENJOY TRASH. THERE'S NOTHING MORE I LOVE THAN TO SIT DOWN IN FRONT OF THE BIG SCREEN AND EAT A BIIIIG STEAMY BOWL OF SHIT. WARNER BROS CAN COME OVER TO MY HOUSE AND ASSFUCK MY MOM WHILE I WATCH AND I WOULD CERTIFY IT FRESH, NO QUESTION

Portland Sucks posted:

Oh god. Sorry, I'm a python developer venturing into a brave new world. Old habits die hard.

Don't be sorry, even Apple's devs got caught by a similar mistake, it can happen to anyone!

john donne
Apr 10, 2016

All suitors of all sorts themselves enthral;

So on his back lies this whale wantoning,

And in his gulf-like throat, sucks everything

That passeth near.

Portland Sucks posted:

Yeah, I'm using the Concurrency in C# Cookbook and trying to wrangle the MSDN documentation. I've done a little bit of basic multithreading in the past, but nothing quite like this before.

Just so you're aware, the async keyword in C# is largely unrelated to threading or concepts of concurrency with the context of a single application. This is good reading if you haven't seen it already: http://blog.stephencleary.com/2013/11/there-is-no-thread.html

SirViver
Oct 22, 2008

I'm by no means an expert on async, but as far as I can tell your problem (besides the lack of curly brace placement) is that you're still running your code synchronously. Specifically you're calling onTick.Invoke(), which is a synchronous call and therefore negates all the async/awaits you plastered around your code.

What I believe you need to do is following things:
  1. Generally kill all usages of async void unless the method is actually used as an event handler which needs to have a void return type. The correct equvalent for this is async Task, so your method returns a Task (without type, i.e. void) that can be awaited. Using async void is very bad form and only exists as a necessary concession to support asynchronous event handlers.
  2. Specifically, change private async void OnTick() to private async Task OnTick(). IIRC you don't need to change anything else, the compiler infers the Task being returned.
  3. Consequently your Action onTick parameter needs to be changed to a Func<Task> onTick.
  4. Finally your call to onTick.Invoke() should be changed to await onTick().
These changes should make your code finally run fully asynchronously.


Personally I think what async can do is quite amazing, yet at the same time I'm not entirely a huge fan of it. It hides very complicated logic behind a seemingly simple syntax that ~~just works~~ except for when it doesn't. Not because it's actually faulty, but because it just doesn't quite succeed in hiding the complexity it tries to hide and inadvertently makes developers assume it does things that it doesn't actually do, which ends up in misunderstandings and perplexing gotchas.

For example, you're using async and at the same time automatically talk about multithreading, yet they have nothing to do with each other. You can use async to asynchronously await the completion of a Task that is executed on another thread (e.g. via Task.Run()), but the main point of async is that it allows for asynchronous code execution without the use of multithreading at its core. How exactly it actually does that I can't adequately explain (because I don't really know beyond a vague idea), but one of the consequences, as far as I understand, is that you can't actually write truly asynchronous methods yourself, unless your async method is just a wrapper that ultimately ends up awating a .NET framework method that implements "true" async. If no such method exists for the things you need to accomplish, the best you can do is wrap the call in an awaited Task.Run() that makes it execute on a different thread - which will work - but will also kinda defeat the point of not using up and potentially starving the thread pool in scenarios where this matters (e.g. ASP.NET). That said, it's at least not going to be any worse than using raw multithreading. I guess what irks me is that the difference between async/await and threading is not obvious enough unless you actually read and understand the documentation (the vast majority of developers will unfortunately not do that when copy/pasting from Stack Overflow is just so much easier), yet at the same time I don't really know how to make it more clear either. Maybe it'll just take time for this feature to sink in :shrug:

SirViver fucked around with this message at 17:44 on Jan 4, 2017

Portland Sucks
Dec 21, 2004
༼ つ ◕_◕ ༽つ

SirViver posted:

I'm by no means an expert on async, but as far as I can tell your problem (besides the lack of curly brace placement) is that you're still running your code synchronously. Specifically you're calling onTick.Invoke(), which is a synchronous call and therefore negates all the async/awaits you plastered around your code.

What I believe you need to do is following things:
  1. Generally kill all usages of async void unless the method is actually used as an event handler which needs to have a void return type. The correct equvalent for this is async Task, so your method returns a Task (without type, i.e. void) that can be awaited. Using async void is very bad form and only exists as a necessary concession to support asynchronous event handlers.
  2. Specifically, change private async void OnTick() to private async Task OnTick(). IIRC you don't need to change anything else, the compiler infers the Task being returned.
  3. Consequently your Action onTick parameter needs to be changed to a Func<Task> onTick.
  4. Finally your call to onTick.Invoke() should be changed to await onTick().
These changes should make your code finally run fully asynchronously.


Personally I think what async can do is quite amazing, yet at the same time I'm not entirely a huge fan of it. It hides very complicated logic behind a seemingly simple syntax that ~~just works~~ except for when it doesn't. Not because it's actually faulty, but because it just doesn't quite succeed in hiding the complexity it tries to hide and inadvertently makes developers assume it does things that it doesn't actually do, which ends up in misunderstandings and perplexing gotchas.

For example, you're using async and at the same time automatically talk about multithreading, yet they have nothing to do with each other. You can use async to asynchronously await the completion of a Task that is executed on another thread (e.g. via Task.Run()), but the main point of async is that it allows for asynchronous code execution without the use of multithreading at its core. How exactly it actually does that I can't adequately explain (because I don't really know beyond a vague idea), but one of the consequences, as far as I understand, is that you can't actually write truly asynchronous methods yourself, unless your async method is just a wrapper that ultimately ends up awating a .NET framework method that implements "true" async. If no such method exists for the things you need to accomplish, the best you can do is wrap the call in an awaited Task.Run() that makes it execute on a different thread - which will work - but will also kinda defeat the point of not using up and potentially starving the thread pool in scenarios where this matters (e.g. ASP.NET). That said, it's at least not going to be any worse than using raw multithreading. I guess what irks me that the difference between async/await and threading is not obvious enough unless you actually read and understand the documentation (the vast majority of developers will unfortunately not do that when copy/pasting from Stack Overflow is just so much easier), yet at the same time I don't really know how to make it more clear either. Maybe it'll just take time for this feature to sink in :shrug:

I really appreciate the write up. I feel like one of the contributing factors is that 90% of the "tutorials", pluralsight courses, even Cleary's book just introduces async/await as a tuple with some super basic examples and then leaves the rest to the reader to infer. Obviously I bring up multithreading because its the only concept I've employed that is in the same realm as asynchronous programming, but yeah I've got no idea how any of this works. I'm currently on the hunt for a MOOC on concurrency/parallel programming from the .NET perspective. Someones gotta be teaching a structured college course on this stuff by now, right?

john donne
Apr 10, 2016

All suitors of all sorts themselves enthral;

So on his back lies this whale wantoning,

And in his gulf-like throat, sucks everything

That passeth near.
I found this series of articles by Eric Lippert to be very helpful in understanding the non-threaded model of C# async. It's basically syntactic sugar around some pretty complex compiler-generated state machine code.

https://blogs.msdn.microsoft.com/ericlippert/2010/10/21/continuation-passing-style-revisited-part-one/
https://blogs.msdn.microsoft.com/ericlippert/2010/10/28/asynchrony-in-c-5-part-one/

ljw1004
Jan 18, 2005

rum

Portland Sucks posted:

I can't get away from the program hanging as soon as I walk outside of wireless coverage. It seems to work fine otherwise, and even works smoothly if I manually disconnected from the wireless network all together and then reestablish a connection.

Other people have already offered useful advice on how to write the async stuff, but I want to step back with some further perspective.

As far as I can tell, there are always some APIs (including asynchronous APIs) that block *synchronously* under poor connectivity. I've never figured out for sure whether this happens in the operating system or in the .NET platform. My suspicion is that somewhere in the OS there's a synchronous block for DNS lookup, and another synchronous block for some part of the wifi protocol. The blasted thing is that you can never *test* these things well -- how do you test with a weak and flakey wifi signal? how do you test handover between two wifi base stations? how do you test a wifi base station that doesn't establish connectivity as fast as it should? and the same for cellphone?

Basically, you have to learn the exact undocumented unexpected behavior of every single API that you're calling, under all situations, despite having no way of testing them...


Another issue is what happens if your fixed up IsDatabaseConnectedAsync() method return true, but then wifi flakes out while you're doing the subsequent steps?


Personally, I think your goal ("work seamlessly in partial connectivity") is a decent user-experience goal. I personally would restructure from the ground up in a much more paranoid fashion...


1. Do your work on a background thread. Never let it run on the UI in case of unexpected synchronous blocking.

2. As a first defense, use the existing APIs to tell if network is connected. I think these are part of WinRT but I can't remember the exact ones. They basically report the same thing as the network icon in the system tray. If these return "false" then do nothing further this tick.

3. If they return that there is connectivity, then don't attempt to connect to the database as a test; just blindly go ahead with your attempted database work, but catch connectivity exceptions.

4. Don't count on any API calls failing quickly. Assume that any network-related API call you make might block unexpectedly for up to 30 seconds. Structure your code defensively to work in the face of this.


More importantly, I'd also worry about user experience. If this "automatic refresh" is supposed to happen automatically, but it hasn't happened for 30 seconds, what can the user do? How will the user know that an automatic refresh hasn't happened for 30 seconds, how will they know the cause of the failure, and what steps can they take? My golden rule is this: the user has better insight into their network connectivity than your code. I think it's important to have a manual refresh button, so that the user can click it to make a database connection attempt right now, and if the connection fails then you should display the precise connectivity error message within 5 seconds. Maybe you'd also display how long ago the last successful connection was, so users can learn to rely on the automatic refresh and don't train themselves into repeatedly pressing this "manual refresh" button every single time they need it.

FrantzX
Jan 28, 2007
Also, be aware of retry mechanics for network connections. If component A will try to open a network resource up to 3 times before it gives an error, and it uses component B that tries continuously for 10 seconds, and so on, things can get exponentially ridiculous very quickly.

dougdrums
Feb 25, 2005
CLIENT REQUESTED ELECTRONIC FUNDING RECEIPT (FUNDS NOW)

ljw1004 posted:

My golden rule is this: the user has better insight into their network connectivity than your code. I think it's important to have a manual refresh button, so that the user can click it to make a database connection attempt right now, and if the connection fails then you should display the precise connectivity error message within 5 seconds.

Unnng yes. I'm a relative luddite and use wifi in my house and office to make calls, and I don't have regular cell service. There are tons of android apps that now assume you're hooked up to the magical cloud fairy 24/7 and will flip out otherwise, and then retry the operation over and over again, forever wasting my battery.

Portland Sucks posted:

Someones gotta be teaching a structured college course on this stuff by now, right?
Not really where I go. I think the trend they're trying to pitch is that you should write everything in a functional language with immutable datatypes and then let the compiler break it up for you. A class I took used F#, and I thought it would be a good project to convert all of the non-recursive functions into opengl kernels, but lo: http://fsharp.org/use/gpu/

The intro data structures courses are all pretty much Java 6.

Unfortunately, the only course specific to the topic of concurrency is a pretty tough (at least for me) graduate course.

Adbot
ADBOT LOVES YOU

biznatchio
Mar 31, 2001


Buglord

dougdrums posted:

Unnng yes. I'm a relative luddite and use wifi in my house and office to make calls, and I don't have regular cell service. There are tons of android apps that now assume you're hooked up to the magical cloud fairy 24/7 and will flip out otherwise, and then retry the operation over and over again, forever wasting my battery.

There really should be some option to allow the phone's network stack to let every network call basically "hang" and do nothing until its timeout instead of immediately returning a failure when the network isn't available. That way at least those misbehaving apps will spend a lot of time effectively sleeping instead of wasting battery in a busy retry loop.

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply