Source Code — Coding Conventions

What the code actually does, measured across the solution — not a style guide anyone wrote down. Follow these to make a change look native; treat the flagged ones as patterns to stop spreading rather than to copy.

Naming

Kind	Convention	Examples
Business class	B<Entity> or <Feature>Handler	BPayroll, BLeave, AllocationHandler
Controller	<Feature>Controller	PurchaseRequestController
Workflow method	Set<Role><Outcome>	SetManagerAccepted, SetHRRefused, SetSent
Scoped query	GetMy<Things>(userId)	GetMyTrainings, GetMyAdvances
Unscoped query	GetAll(...)	GetAll
Permission key	<Area><Action>	PurchaseOrderDetails, StockSRR, HumanResourceIndex
Approval routing	Get<Doc>NextUser	GetPrNextUser, GetLeaveNextUser
Workflow constants	<Doc>UserTypes / <Doc>OpTypes	PRUserTypes, LeaveOpTypes
View model	<Feature>VM	TrainingVM, ResetCheckVM, DeductionVM

Braces are Allman. Private fields are inconsistent — db, _user, userObject and _db all appear, sometimes in the same constructor.

Known spelling irregularities

These are load-bearing: fix the spelling and the build breaks.

Spelled	Should be	Where
AttenadancesHandler	Attendances	class name, NKO.Shared/BAL/
SetcountryDirectorAccepted	CountryDirector	BTraining
SetCountryDirectoreRefused	Director's	BAdvance
PerndingForSrrId	Pending	Asset entity
IsStockAvailabeInWarehouse	Available	BStock
GetRefrenceCode	Reference	EvidencesController
ExportPendingAttendences	Attendances	AttendancesController

Async

2 090 public async Task methods. Only 30 end in Async — about 1.4 %.

public async Task<ActionResult> Details(string id)        // the norm
public async Task<AppUser> GetUserNameAsync(int id)       // the exception

The suffix is used only where a sync overload of the same name already exists (GetContract / GetContractAsync). So: do not add the Async suffix unless you are disambiguating an overload — a new FooAsync would look foreign here.

Consequences worth knowing:

• There are two sync-over-async sites, and no .Wait() anywhere: BExchangeRate.cs:34 (ReadAsStringAsync().Result) and PayrollController.cs:85, which calls Task.Run(() => userObject.GetName(...)).GetAwaiter().GetResult() inside a LINQ projection — one thread-pool hop and one blocking wait per row of the payroll grid.
• ConfigureAwait(false) is used exactly once in the whole solution — BAL/BExchangeRate.cs:31, on the line above the .Result cited just above it. Everywhere else it is absent, including in the class libraries.
• CancellationToken is accepted only by the IHostedService jobs, which the framework requires.

Error handling

catch (Exception ex) appears 948 times. Of those, 48 have a completely empty body (another 10 hold nothing but a comment):

catch (Exception ex)
{
}

The dominant shape is catch-and-return-a-sentinel rather than catch-and-log:

try
{
    // … work …
    return model.ExchangeRateId;
}
catch (Exception ex)
{
    return -1;                 // caller cannot tell what failed
}

Pattern	Effect
return -1 / return null / return false	Failure indistinguishable from a legitimate result
Empty catch	Failure invisible — EmailManager (R1-6)
return RedirectToAction(..., new { errorMsg })	The user-facing pattern; the exception itself is lost

There is almost no logging. No logging library is referenced, and ILogger<T> is injected in only two business classes, BStock and BAsset — 27 LogInformation progress messages in the warehouse send flow, and not one in a catch block. The only other logging configured is options.UseLoggerFactory(LoggerFactory.Create(b => b.AddConsole())) on both DbContexts, which logs SQL, not application events. Combined with UseDeveloperExceptionPage() running in production (R1-1), a production failure is visible to whoever triggered it and to nobody else.

When adding code

Follow the surrounding try/catch shape so the file stays consistent, but do not add a new empty catch. If there is nothing useful to do with the exception, letting it propagate is strictly better than swallowing it — at least until logging exists.

Data access

• No repository or unit-of-work layer. B* classes take DbContext and write LINQ directly.
• No interfaces for 40 of the 44 business classes — they are injected as concrete types.
• Lazy loading is off. Microsoft.EntityFrameworkCore.Proxies is referenced and navigation properties are virtual, but UseLazyLoadingProxies() is never called. Every navigation a query needs must be .Included explicitly, or it comes back null.
• IgnoreQueryFilters() is used where soft-deleted or draft rows must be visible, e.g. HumanResourceController.DraftEmployees.
• Scoping is applied by hand, inside each method, via BUser.GetPermission(SharedPermissions.<Area>, userId) and a switch on DataPermissionType. The same ~10-line switch is repeated across many classes.

Controllers

• Thin by design: resolve the user from CurrentRequestData, call one B* method, pick a view.
• Paging on every grid via X.PagedList, with a shared parameter vocabulary — int? page, string search_param, string searchString, string currentFilter, string sortOrder, int? filterYear, int? filterMonth, int? filterStatus, int? filterBranch.
• Details / Edit / Delete take a GUID string; grids and lookups take int id.
• Every multi-step document exposes exactly one ContextActions POST driving the whole state machine, instead of a verb per transition.
• Failures are expressed as a redirect carrying msg / errorMsg, not a status code.

public on a controller method makes it a route

MVC routes every public, non-static method on a controller. Eight internal helpers are reachable over HTTP purely because nobody marked them private — including AccountController.SignIn (R3-7). Default to private for anything that is not an action; use [NonAction] if it must stay public.

The mirror image: public static methods are never routed, which is why the seven GetMonthDifference copies are duplication rather than endpoints (R3-3).

Language level

All five projects target net6.0, but modern C# features are effectively unused:

Feature	Status
File-scoped namespaces	0 files — all block-scoped
<Nullable>	Enabled in NKO.Helpers only — the 1-file project
<ImplicitUsings>	Enabled in NKO.Helpers only
Records, pattern matching, init	Not used
Minimal APIs	Not used — classic Program.cs + Startup.cs

A file moved between NKO.Helpers and anywhere else changes meaning: nullable warnings appear or vanish and using directives become required. Tracked as R8-5.

Structural oddities to expect

Oddity	Detail
Namespaces ignore folders	Helpers/BUser.cs is NKO.BAL; Configs/ holds two different namespaces. See Solution Structure. R8-1
Hangfire/ holds SignalR	RealTime, RealTimeNotifier, CustomUserIdProvider. R8-2
TrainingStatus.cs holds seven modules' enums	LeaveStatus, AdvanceStatus, OvertimeStatus, UpgradeStatus, … R7-15
Two filenames end in a space	AdvanceController .cs, TrainingController .cs. R7-14
A helper derives from Controller	ExportHelper : Controller — not routed (the name does not end in Controller), but it inherits the whole MVC surface to reach view rendering
Multiple classes per file	FilesController.cs also defines PublicFilesController; ClaimRequirementFilter.cs defines the attribute too
Warehouse split by folder, not project	WareBAL/ and WarehouseControllers/ live inside NKO.Main

Testing

There is no test project. No xUnit, NUnit or MSTest reference exists in any .csproj, and no *.Tests directory. Every change is verified by running the application.

The two things that would have to change first to make tests possible: business classes take a concrete DbContext (so there is nothing to substitute), and 40 of 44 have no interface. Tracked as R8-4.

Checklist for a change

• [ ] Rule change → the one B* class, not the controller.
• [ ] New service → AddTransient in Startup.cs, concrete type, check it is not already there.
• [ ] Non-action controller method → mark it private.
• [ ] New query → .Include() what you need; lazy loading is off.
• [ ] Row visibility → call GetPermission and switch on DataPermissionType.
• [ ] New approval step → NextUserHelper for routing, Set… on the B* class for the transition.
• [ ] New constant → CommonConstants.cs, or OperationConstants.cs for warehouse.
• [ ] Don't add the Async suffix, and don't add an empty catch.
• [ ] Verify by running the app — there are no tests to run.

Related

• Solution Structure · Core Classes · Services & DI
• Architecture — Request Pipeline
• Recommendations — R8-1 … R8-6.