Codebase Problems - GibraltarSoftware/Loupe.Agent.Core GitHub Wiki

The current codebase is an inflight refactor and reinvention of the Gibraltar.Agent originally developed for .NET 2.0. As a result, it suffers from a number of problems that have yet to be addressed. In most cases the problems don't prevent Loupe from working but are sub-optimal for a modern application. These can be picked off and worked on as there are resources available and not all are going to be addressed before shipping Loupe.Agent.

The original codebase adhered to several style aspects that were common in .NET 2.0 (think 2007...) and have since dropped out of fashion. The Resharper settings file indicates the desired coding style going forward but significant parts of the codebase do not comply with this yet. For example:

Blame the COM roots of several of the original Loupe development team, you'll see a lot of variable prefixing indicating scope of the variable. For example m_OurClassVariable. Then, for consistency other prefixes were used to designate static variables and thread-local variables (g and t respectively). The accepted convention going forward is to use no prefix for global, not bother differentiating thread local in the name of the variable (but DO put a comment on the declaration) and use a simple _ for member variables.

The current implementation contains all of the code necessary to read session data and write it. This causes a few issues:

• There are a substantial number of classes in the API that are only used for reading and will likely never be used by any client. They can only cause confusion.
• Invalid Operations are possible using some shared types that are used for both reading and writing depending on the mode. These methods could be completely eliminated if reading and writing code was separated.
• Significantly larger distribution size and porting requirements for all of the extra types.

While we do have a goal that anyone can write an application that can read Loupe data in full fidelity having this intermixed in with the writing code isn't optimal. Instead, we want to factor out the reading code into a separate assembly so it can be referenced independently and only used by applications that want to read serialized data. This can reduce the size of most applications using Loupe (a particular concern with mobile applications) and potentially be supported on a more narrow set of platforms than the writing code is supported on.

Loupe has its own API syntax for methods that are asynchronous from the caller, complete with custom return types and methods for doing continuation and monitoring results. These are unnecessary with modern .NET Async support and will cause confusion due to nonstandard use of async terminology. Since Loupe Agent is only targeting newer version of .NET where async/await is available all of these areas should be modified to adopt a more standard approach using the TPL and async/await where appropriate.

Loupe uses Dictionary<K,T>, Queue, and List extensively. Since it is also widely multithreaded there are custom locks written around this code to ensure it works correctly. In several cases this complexity can be dropped by using an appropriate concurrent collection object. These are typically much faster than using the equivalent generic collection as they are designed with lockless internal semantics. To make the entire Loupe codebase more approachable and to improve performance migrating to these equivalents is desirable.

In cases where there isn't a performance advantage or usability advantage to the end client of Loupe it's fine to leave the current generic collection item in place.