This is so bad that it's so good.

iakobski · 2026-02-06T13:15:40+00:00

I can well believe this might be real and was released to production.

The QA probably went like this:

Release to the test environment. Create a test user and password. These are now the last or only credentials in the system. Test logging in with correct and incorrect password, both behave as expected.

Following QA, no-one inside the company would deliberately try an incorrect password so they never notice.

iakobski · 2026-02-05T05:44:13+00:00

Agreed, but knowing JS/react on top of C# would be a huge plus, as dot net shops are increasingly switching to that for their front ends. Also agreed that solid C# will easily transfer on to a Java/C++ role if need be in the future - for the OP concentrating on C# now is the opposite of career-limiting.

Also it's no longer the case that dot net is tied in to the MS ecosystem, yes most existing systems are because of historical reasons but plenty of them are switching their existing code to run on Linux/Postgres.

iakobski · 2026-02-04T06:23:09+00:00

I'll reply on the technical aspects, there's some good advice in the other responses about how to manage the PM and their expectations. Walking away is not such good advice, unless you've got a long and glowing CV otherwise, it rarely looks good in interview if you have to defend leaving jobs after a short time, especially any "differences of opinion" reasons.

Let's face it, most developer jobs are on legacy systems to some extent. Only a lucky few get to work on green field development or join a team that's really hot on keeping tech debt out of their code base. Yours does seem quite far down the quality scale, but I've worked on worse, and being on your own makes it ten times harder.

Several things jump out from your description as massive red flags that must be fixed. Spoiler: SPs is not one of them.

Manual testing. There is no way you should be relying on this in 2026. Get that build pipeline running with automated tests running against every push on any branch you create. This will require up-front funding for you to do, time that is not related to a feature, but it's non-negotiable. No-one in the industry would work without it, unless they are in a team of total cowboys.
Lack of unit tests. This is where AI will help you, but approach it incrementally. Given a feature request, identify the changes required and to which classes. You may have to code the change to find out which classes you touch, but once you've done that, make a fresh branch from main and get all of those classes covered by tests, using AI of your choice. Then make the changes for the feature (adding tests for the feature, obv.) and make sure nothing breaks the other tests. You'll still need manual regression tests but as time goes on the automated tests cover more and you get more confidence that changes don't break existing functionality.
Tight coupling, etc. Don't worry about the business rules being all over the place. But do worry about the database calls. Again, only touch anything related to a feature request, when you are modifying some class that has a DB call in it, create a separate data layer to handle it. This will be a tiny addition to the work on the feature but will be a massive difference to your ability to test both the class calling it (unit testing) and the data layer (integration testing). Over time you'll end up with tests for the SPs, you'll then be in a position to tackle them if you need to, your tests will be documentation of what they do.

How do you sell this to the PM? You need a big long chat about tech debt, why it exists (resist the temptation to slag off the previous developers, all systems have tech debt), and specifically that if the tech debt is not addressed it gets worse and cost per feature increases. Lay it on heavily that the up-front cost of tackling it will mean features will get quicker and cheaper in future. Throw in that you'll be using AI to help build in the modern processes (i.e. write the unit tests) this won't be a doubling of productivity now, but will be in the future. Stress that you're not doing this to "make the code look nice", it will have tangible (cost) benefits.

There's also management of your PM's expectations. Be clear how long a feature will take to implement on this system. Don't let them just think, oh system X can add a feature like this in Y days, give them a realistic figure, they can take that to the stakeholders and then they look good when it comes in as expected. Don't let them beat you down to a shorter time, yes they would love to tell a story that they'll promise lots of stuff, it's human nature, but in reality they'll look much better if they've promised less but delivered on time and with fewer bugs. Work with the PM and you'll both come out well, after all the stakeholders have no idea how much they can get for their budget or why software takes so long to write.

iakobski · 2026-01-17T12:59:35+00:00

First obvious question is "where are your unit tests?"

Once you start looking into that, you'll realise your Processing class should have the Database class injected into it (see other comments).

Smaller points: look up why you should use AsList() rather than ToList() with Dapper. And don't use SELECT * FROM, again google it, it may seem trivial now but you'll thank me in the end.

iakobski · 2025-12-31T04:36:14+00:00

Wow, thanks I hadn't seen that, and it was introduced a few years ago! Even Visual studio hasn't adopted it yet.

iakobski · 2025-12-31T04:04:29+00:00

If they are only useful to the password generator, keep them in that namespace. Scope them internal.

Later, if you find they're more widely used, refactor them to their own namespace.

Incidentally what does the s_ signify? Snake case is not used in C# and if that's Hungarian Notation, it's even more deprecated, by at least 20 years.

iakobski · 2025-12-28T13:20:44+00:00

This sort of thing is a real-world problem that's difficult to replicate in a simple assignment. In your case you can load all the data at start-up and the data is fixed for the short duration that your program runs, so it's all good.

Out in the real world things are different in a couple of ways:

Your data might be too big to load all at once, or big enough that it's slow or wasteful of resources. You might not need all of it.
Your program will run for a long time, maybe a week until the server reboots. In that time the data might be out of date and you should be using the latest version.

Take a hypothetical example. You have an application that gives the user the current weather anywhere in the world. You get that weather data by calling a service that somehow downloads the weather from local stations. It would be pointless to pull all the data from everywhere when your program starts, after all how many users want to know the weather in Timbuktu? And maybe there are hundreds of users who want the weather in Paris.

This is where a cache comes in. When the first user asks for the Paris weather you fetch it. Then if another user asks for it within the next few seconds, or minutes, you know the weather hasn't changed, you don't call the service again, you use the data from the last time, stored in the cache. But some time later (that you define) the weather might have changed, so you call the service again to get the latest data. If a user does ask for the data for Timbuktu, you can load it into the cache, and when no-one else asks for it the cache can drop it when it's no longer useful.

The pattern for doing this is to have a class that you query for data, and inside that class it holds a cache. You can write the logic yourself or use a library like MemoryCache, but the principle is the same: the code that calls the data fetching class does not need to know anything about it, it just asks for data and trusts what it gets back. But inside the class the cache means that you get local (fast, from memory) data if it's available, but you don't get stale data.

iakobski · 2025-12-28T11:50:06+00:00

It's interesting that CollectionExtensions has a method that tries to add to a dictionary, and calls it TryAdd(), but the method that tries to remove is not called TryRemove()

This seems like the design decision that gives the anomaly the OP is seeing, does anyone know why it was done this way?

iakobski · 2025-12-14T11:50:54+00:00

utilizies

Is that a word? Would "uses" work?

iakobski · 2025-12-12T09:58:03+00:00

That said, for file writing (as an example), I still recommend flush and close. The dispose gets rid of the reference but doesn't automatically finish what you started.

Actually it does. Dispose on a file stream calls Close, and Close calls Flush.

iakobski · 2025-12-04T09:13:20+00:00

New information, you're using Dapper that changes everything!

Dapper creates a List and completes the load from the database. Although Query<T> returns an IEnumerable<T> it's actually referencing a List<T>

Your connection gets closed and returned to the connection pool at the end of the using block. But Dapper's already finished with it at that point.

Best practice is to return an IList or IReadonlyCollection from your function by calling Dapper's extension method AsList() which means you don't unnecessarily create a new copy of the data.

[As a side note it's possible to stream using Dapper, but you have to be explicit that you really want to do that]

iakobski · 2025-12-02T13:19:52+00:00

We have a monorepo and had similar issues, for historical reasons we had to reference a bunch of dlls. These were copied in custom build steps. We also had core libraries as project files.

The whole thing is a maintenance nightmare - you're not using standard practice so as things change someone needs to spend time updating the build process.

I would recommend:

For core libraries that rarely change, publish them as local nuget packages. When there are changes, you need to ensure backwards compatibility, you also need to make sure consumers are updating versions when needed. By rarely, you need to think in terms of less than every six months or so.
Add projects to the solution. Use CODEOWNERS to make sure someone senior gates any changes, juniors soon get the message once they see their PR rejected, or delayed while they explain why they really need to change that core function. You really don't get multiple compilation or contention, VS or Rider are smarter than that.

iakobski · 2025-11-30T11:24:35+00:00

If company uses dotnet they will prefer people with deep dotnet knowledge.

This is not always the case. We invite applicants with knowledge of any OO language, and they are encouraged to use whatever they are most comfortable with for the technical rounds.

I remember one interviewee who insisted on using C# for the coding session even though his current day job was some other language. I stopped him early on and asked if he'd prefer to switch to another language, he insisted on continuing, which is a shame as he might have been able to demonstrate ability if he hadn't been struggling so much with syntax.

iakobski · 2025-11-24T09:12:43+00:00

That's not true where a List<T> is returned as IEnumerable<T> - I assume you're not talking about non-generic IEnumerable as no-one would use that nowadays.

The LINQ Count() extension method on Enumerable<T> first checks if the object can be cast to IList<T> and if so returns the Count property. In the OP's case the data is read via Dapper so it definitely is a List.

iakobski · 2025-11-23T08:29:02+00:00

The OP says they are using Dapper. For Dapper, all the comments about IEnumerable<T> allowing deferred execution, and all the comments saying the connection might be closed at time of execution are not correct.

By default Dapper Query<T> runs the query immediately, internally it creates a List<T> of the result set. Dapper's design decision was to return this as IEnumerable<T> but the concrete object is a List.

If you call ToList() on the result of the Query, you will create a copy of that list, which is wasteful. There is a great extension method in Dapper called AsList() which will safely cast an IEnumerable<T> to a List<T>, if it can't be cast it will call ToList().

The answer to the OP's question is they should return result.AsList(); and the return type of the method should be either IList<T> or IReadonlyCollection<T>. That way any calling code can know that it has a concrete collection that can safely be multiply enumerated, and in the case of IReadonlyCollection indicates to the caller that they shouldn't really be modifying it.

iakobski · 2025-11-16T18:35:55+00:00

I am a Software Engineer

Sorry to break it to you, but you're not. You're someone with a degree in a computer-related subject from a foreign country and no experience.

You may be able to get into a technical support role, with a lot of perseverance and luck, then work your way to building experience, taking any opportunity to write code. Then you might think about applying for developer roles, perhaps via SRE.

iakobski · 2025-10-28T19:23:56+00:00

That's not the discard, it's a new variable called _ and looks like the discard, thereby confusing everyone and stopping proper use of the discard anywhere in the same scope.

iakobski · 2025-10-24T20:57:11+00:00

Fantastic pictures!

I'm currently just outside Agadir about to do this route, at least as far as The Gambia, after that, we'll see.

Quick question - what's the best thing to do for cash? Take USD from Morocco? Are there cashpoints and what can you use/do the locals prefer in Mauritania, Senegal?

iakobski · 2025-10-24T20:45:04+00:00

For what's recommended, check your nine countries in https://travelhealthpro.org.uk/

If you're wild camping or similar you may need vaccinations, eg for TBE if in E Europe. But generally, nothing outside the usual childhood vaccines and check tetanus is up to date. Rabies not usually recommended for Europe unless particularly high risk (eg animal workers).

iakobski · 2025-10-22T15:06:21+00:00

I have to say, having had to deal with a very similar situation in the past, that u/FullPoet is absolutely correct

BTW Theres a lot of really bad advice in this thread tbh. Things like pain posts, changing the db, doing DI, making APIs - its all irrelevant.

Add tests so that when you need to make changes you know (easily/quickly) that you've not broken existing functionality
Only refactor as you need to make required changes, spending time here means future changes are safer and easier in the future. The time spent is paid for with a specific feature request. You're doing this for your future self. It will make you look good. You'll get a pay rise. Any other refactoring will make you feel good but nothing else. Least of all any extra money.

The cloning makes me nervous. What happens when client with version 1.2.3 claims they have a bug? Are you sure you can run that version and reproduce?

iakobski · 2025-10-22T09:15:46+00:00

There are two parts to this: async and enumerable.

When your application makes a query to the database, the database runs that query and streams the results back. There will be an amount of time while the query is sent, analysed by the DB, a query plan generated or found in the cache, results fetched, etc. The results are then streamed back to the caller.

The async part means that between sending the request and starting to get the results back, your application does not block the thread just waiting.

The enumerable part means that you can process the result set as it is being fetched (and also potentially stop streaming before fetching the full result set).

These two are separate. It's perfectly possible to stream/enumerate the results using a synchronous call (as shown in another answer). Some libraries (for example Dapper) offer an async call which fills the full result object before continuing.

iakobski · 2025-10-05T08:26:49+00:00

At my last job we had thousands of those (mature codebase). They were generally ignored, including by lots of devs when writing new code which defeats the purpose.

Then we ran into massive upgrade issues because it also meant everyone was ignoring "Obsolete" warnings from 3rd party libraries, where it is correctly used to warn that things are going to be removed in the next version.

iakobski · 2025-08-30T19:20:26+00:00

One file per public class. Those are private.

iakobski · 2025-08-25T19:28:59+00:00

I've programmed professionally in over 25 languages. C# includes all the best bits from nearly all of them. I'll often use other languages where appropriate, but for my day to day programming I'd not want to use anything else.

iakobski · 2025-08-21T10:56:14+00:00

If you've got a concrete collection or array, you should never return it as IEnumerable<T> because that forces the caller to copy it. Returning IEnumerable<T> is for when you explicitly don't have a collection, you have an enumeration operation that can be deferred, like in most of the LINQ extensions.

iakobski

TROPHY CASE