"I Lost a Job Because of This Codebase"

[@henryvaneyk3769]

I am a Tech Lead, and nit-picking like this is stupidity at best. I never expect an applicant to do the work exactly as I want it. My requirements are simple. Did the work satisfy the acceptance criteria? Is the code readable and does the project structure make sense? There is no need go into more detail as each team's way of doing things differ. The most important part for me is the attitude of the applicant, in other words, will I enjoy having the person a part of my team. The other smaller details can be sorted out when onboarding the applicant into the team.

[@Prod-23]

Finally!!! Someone said one of the most important things "will I enjoy having the person a part of my team".

[@ARumGremlin]

I absolutely agree. I've worked with enough companies to realize that rigidly adhering to dogma gets you into more trouble than it helps. Ultimately it boils down to whether or not it meets requirements and other team members can easily understand what the hell is going on. If a dev can do those things, anything else can be retrained to meet your team's specific best practices.

[@sealsharp]

There's a difference between "thats how we do it at this company" and "oh damn boy, you didn't guess correctly how we do it at this company!". How should they even know?

[@asagiai4965]

I agree, especially the last part. I think you might also want to add "how helpful he is"

[@nunofigueira8691]

Thank you do be a good lead and someone that really know how the real work is😂😂😂

[@powerclan1910]

i think you judged many things that are 'opinions' and 'choices of preference' as absolutes, this is in my opinion a very bad way to do code review, as there was very minimal communication about the preferences. how you name things, using a folder structure that everyone used 7 years ago, or using the default provided framework when it is sufficient for a sample project are in my opinion not bad things. they show you can follow standards, you just disagree on them and as long as they are willing to adopt new standards, there is 0 issues there. Same for what content is returned from api calls, 90% of companies don't follow exact REST guidelines, and some even have their own. The error handling and how he used entity framework are the real issues here, almost everything else is a matter of opinion that is different for each company, and should never be red flags

[@metaltyphoon]

Seriously. Nick loves to put Async suffix while ASP NET Core ditched this long ago. Does it mean he’s a “NO” when applying?

[@Divus90]

Also, good luck finding one single "REST guidelines" standard. It's not like http or auth, that are actually standarised. It all depends on the style of code people are used to. I hate when people create layers when they are not yet needed (Service layer in this situation, which doesn't do anything right now, only hides repository), and would ASK about this on the interview. Generally I hate coding tasks before interviews that do not qualify for an interview. This industry is cursed on the notion that people are often searching for developers with the same opinion about coding practices, and not looking at objective issues.

[@Downicon3]

Agreed, he picked out stuff like assertions, really? Endpoint lowercase

[@AmonAsgaroth]

I agree, but that being said, most interviewers will also absolutely do that, so you have to prepare for it on a job hunt. It's just the dumb reality we are currently facing. I guess it makes a little bit of sense if you have a surplus of candidates? (Like now). You can just pick a person who already follows the company convention, regardless if it takes 15 minutes to learn it I guess.

[@nunofigueira8691]

It is a shame... Doing a Code Review and provide feedback based in personal opinion against to scientific issues like CPU performance, database impact and low performance, security risk 😂

[@emmanuelrf]

Hi Nick, I'd really like to see your implementation of this take-home assignment from scratch. I bet it would make a great video for all of us.

[@RonaldCabreraGonzalez]

I agree with this one.

[@jebotipasmater]

I second that as well, this is a really popular type of take-home assignment - CRUD API. I had a bad experience recently where I didn't even get a normal feedback from a company about what they expected to see, just that my implementation is not at the level they expected. For the time I invested I would certainly expect a decent feedback, that's all I can say. Also, I will start charging for these assignments, personally, I won't be doing them for free any longer. I'm too old for this 💩.

[@FernandoTakeshiSato]

yup, would like to see that. Then in a few months, it's all shit because it's not using latest and greatest...

[@nickchapsas]

Sounds like I have to make this video then

[@kevinlyman1]

I agree. I'm constantly feeling like an imposter and there are several things in this video that I've done or do that way now. I wish there was a simple "right way" in software engineering. I'm hoping Nick's way is the right way.

[@potentiallyRealWarrenGraham]

Alright, if Im ever fired from my current job im fucking cooked lmao

[@leftjabrighthook]

Same. They fucking have us where they want us. Living in fear, never asking for raises, basically abusing us.

[@DukensteinA1]

I feel the same way. 💀

[@ARumGremlin]

My contract is up on Friday, guess I'll go apply at the local grocery store. :(

[@da3dsoul]

If you have more than 2 years, just tell them no. We're all adults here. I have plenty of recent public code. You can open my most active project and look at it. You'll learn a lot about someone from what they choose to do for free, as well.

[@SilasPeters]

If only Blackwell Academy had a computer sciences course

[@pawegruba4719]

If no one told you to create many projects and your application was rejected due to the lack of such a structure, you know that you have nothing to lose because you will not learn anything valuable in such a team.

[@daniellessio2323]

I get your point and I kinda agree, but depends on the context as Nick said, if it is higher than a intern or junior position how much do you want or should say/specify? Some best practices are expected in certain positions, and given them away as "condition to pass" removes the point of the challenge. Or maybe you are expecting the person to ask these kinda questions before diving in the code and that was part of the test. The "challenge" itself is a very basic API in the end.

[@jebotipasmater]

The problem here is that it's a gatekeeping issue with these steep criteria. Normally once you're in the team you can kinda relax about this because such minor issues get rectified during code reviews. I say minor because IMO just about everybody can learn the current 'proper' way of coding. I mean for example it takes a .gitignore file to fix committing bin files and such. It's not a rocket science.

[@pawegruba4719]

@@daniellessio2323 I can't agree with you, no matter if it was junior, mid or senior role, if no one told you to split it in multiple layers, there is no need for doing that (because it is a simple task). Second point, if you get such task for senior role - run away, I would never interviewed senior in that way. Thirdly, if any interviewer gives a task without explaining the goal, again - it won't be a good place to grow. I got a similar task a couple of years ago, but the interviewer told me to show off as much as possible. So there were layers, middlewares, handlers, patterns, etc, overengineering at full gear - but the goal was clear, show me what you know. Another time I got a task to solve a problem, in the simplest way I was told. Everything was up to me, the only "goal" was to pass predefined tests. And I failed, getting a similar review as in this example - but tests were green :)

[@damianradinoiu4314]

​@@daniellessio2323you are wrong.. some aspects are subjective in programming... that's where the problem lies. If i were to ask for an interview to design an API with 4 endpoints and the person comes up with an over-engineered app (adding monads, mappers, resiliency etc) i would say that IS a big mistake because it means that person doesn't care about maintainability and just learnt to copy/paste some structure he has seen previously. If i explicitly told him "hey.. I expect this project to contain that and that and that" then ok fine... Let me ask you, have you ever asserted anything without specifying an expectation ???

[@daniellessio2323]

​@@damianradinoiu4314 sure, as you said, somethings are subjective, like conventions. But some are not. Let me ask you this, if you are tasking a senior dev do you specify that unit tests are expected? If the dev sends a PR without it would you approve it because you haven't asked? Both ways of thinking can be right, as I said it depends on the position, and maybe they were expecting those questions from the candidate.

[@MrFallout86]

I've been developing for 15 years and I've worked from governmental to high demanding financial sector projects. There is absolutely nothing wrong with naming a folder "Interfaces". Please don't label your subjective opinions as objectively correct approaches. Getting hung up on folder names is very silly. As long as it makes semantic and logical sense and it's not too embedded, it's fine. Also again, to say because you (possibly due to a typo) have a singular rest api end point name, "you don't understand RestApi" is a bit nonsense. I've worked with some phenomenal developers, who would easily use the [controller] syntax for a controller. What metric is that to judge someones ability to solve a large-scale problem. Getting too hooked on these issues I genuinely believe makes you a weaker developer. Focus on the stuff that matters. That being said, this repo is filled with errors. And some you've identified correctly.

[@jcx-200]

Yeah I would agree on the folder names and controller routes. I'm talking about this from a far less experienced position (2.5 years) but even if these were supposed "red flags" for an employer, they are very minor things that ultimately come down to personal preference that can always be clearly communicated to a canditate on hire about what their coding standards are surely.

[@josefromspace]

Agreed.

[@duramirez]

I agree 100%

[@duramirez]

Well in a Interview test I agree its silly to look for such details. But if it was an employee that already knows the Company standards, thats a different story, its not silly anymore, altho I do allow my developers do be creative and do things their way, I expect them to follow the pattern already established and a simple singular instead of plural name, would put their work into the Refactor tab in my Jira. hahahaha

[@jcx-200]

@@duramirez On the job is for sure different and an elevated level of scrutiny is valid. It just really does not translate well to an interview.

[@martintsekov7820]

More reviews please! That was quite interesting to watch.

[@willog87]

I was going to write this exact comment! It's good to see imperfect code and see what we can learn from it.

"I don't really like ... therefore it's a red flag". Sorry but judging on personal taste is bad practice. If you want to do so give them your conventions. Btw: What the heck is the objective of task like that? To see if the person is willing to invest some time into the process? Total waste of time for both parties. I much prefer to have 1-2 hours of pair programming with the candidates.

[@bogy912]

that's probably the way how it should be done in the interviews like this. especially for junior positions. these homework stuff is tetlling pretty much nothing imho :)

[@Thial92]

I had a similar situation once many years ago. I was asked to write an API with some endpoints and a database using any method I like. I thought to myself that they want to see if I truly understand how networking works so me coming from a broadcasting backend background I wrote a very low level API to show that I really understand how it all works. I was rejected and told that they wanted me to use Entity Framework and controllers. I was like why do you say any method if you expect me to use something specific not to mention that the way I wrote it really showcased my knowledge instead of just using frameworks which I could have done in 15 minutes. That day I learned to not be fancy because everyone wants to see the most mainstream solutions.

[@duramirez]

I would have passed you because of that, because yeah, I don't like following mainstream architectures just because. I do follow them, but I have to agree. 🙂 In this sense for example I do not agree with the video about having folders with specific names, I do call my folders Models, Repositories, Controllers, etc. For me it is much easier to find stuff this way. But some other mainstream "rules" I don't follow. I don't think I am obligated, no matter the size of the Company I am working for, I have to follow whatever standard the Company define, and if I am the one defining, I will follow my own logic and tastes, because I had to deal with other people's tastes for 20 years in my career, so screw whoever think they should not do things based on taste. hhahahahahaha

[@BeekuBird]

Recent jobs I've been in have take EF out.

[@Fafix666]

Why? Because certain "tech leads" are daft fucks who can't think or work in any other way than they've been taught. I call it an Indian Developer Syndrome. These guys will throw a hissy fit if you do anything remotely different from what they consider a "standard". Just a frail ego thing.

[@tiagovallejo]

Those are the same people that write backlog tickets and acceptance criteria 🤣

[@dovh49]

I'm getting to the point where I don't even want to do programming anymore. Everyone wants React/Angular experience. I know JS/HTML/CSS really well and understand that those SPA frameworks are actively harmful to companies. But everyone wants conformity. If you think too hard about the subject then you are ejected.

[@WarpingWombat]

Honestly I As a Senior Dev favor Putting everything in one Project for simple Projects structuring it with vertical folders

[@AlgoristHQ]

Same. I called it organic growth. If I run into an actual reason to add another csproj, then I add it then. Don't add it at the very beginning just "because."

[@Rick104547]

Yeah exactly, creating a new project should have a valid reason. For just structure you use folders.

[@Denominus]

I’ve been advocating for this… for like forever. But it’s normally a really unpopular opinion in the .NET community. Since I started with .NET 18 years ago, it’s been widespread. Projects are a physical separation, not logical. They are for code sharing and executables.

[@flibbertigibbet6324]

There are multiple valid ideas on project/solution structure and the right structure for a development team is largely derived from cultural history within an organization. The interviewee could not divine what was right for this employer through a simple endpoint coding test.

[@neutralenull]

@@Denominus I dont get it either. It is not like most companies who jumped the ship are reusing parts of the application anyway. Still i have adapted but under protest lol

[@sealsharp]

Interview: You are not doing it the correct modern state of the art way, like newtonsoft, who uses newtonsoft? Job: Your first task is to fix a bug in this ASP app. That? No, thats not c#, that's VBScript. Btw. did yo learn COM interop in school?

[@asagiai4965]

...ahh I'm using newtonsoft

[@ABC_Guest]

So true

[@stoic7810]

.net's json serializer is not yet as fully fledged as newtonsoft. so i still use newtonsoft.

[@davew2040x]

Sounds like this candidate should have just had ChatGPT write it. Utterly useless waste of time. These “write a minimal project” at-home tests are practically designed to devolve into the reviewer nitpicking about whether a candidate happened to use the exact same set of boilerplate standards and practices that they use at the company, which isn’t much of a measure of an engineer. It certainly guarantees that you’ll never hire anybody except people who have worked in places that are an exact mirror image of your current environment. If they wanted to be fair about it, they’d provide the company standards documentation along with the test. Spoiler alert: They don’t have a document like that, it’s likely just one guy who loudly declares “everybody knows you do it like this”.

[@BittermanAndy]

I'm curious what you think companies should do, then. Leetcode? Everyone knows that's a terrible approach, it tests whether they practice Leetcode not whether they're any good at the job. A basic coding test that tests their understanding of how to write code? You're saying that's worthless. So, real work, then? That'll take days or longer of their time and they'd need to be paid for it (so would need to set themselves up as a business, inform the taxman, tell their current employer about a potential conflict of interest, etc.). Or maybe you think just take people's word for their skills - have you ever worked with someone with loads of experience who wasn't very good, or interviewed someone who lied about their experience? To me, a basic coding test is by far the best option. But I'm all ears to hear which of those choices you think are better.

[@davew2040x]

@@BittermanAndy I’d say that essentially anything is better than “do you write boilerplate the same way I write boilerplate”. Provide some kind of interesting wrinkle to implement other than “fill in the blanks on an API”. I would even say that LeetCode is more useful. “Does this person know our style guidelines” isn’t indicating a whole lot about anything.

[@BittermanAndy]

@@davew2040x an interesting wrinkle, I'll give you that. The tests I've given people generally tend to ask them to calculate something (checking basic correctness, spotting edge cases, and simple performance concerns). Just writing a CRUD interface isn't very interesting, I grant you. But then the risk is that anything too complicated and it takes too long to do, or is unfair to those unfamiliar with that kind of maths, or strays towards being Leetcode. But I'll always ask someone to write code, always.

[@bryanedds8922]

bro, do you even boilerplate?

[@mikewright2858]

After 30 years doing this professionally, and writing code starting with 8 bit assembly language in 1982, my answer to any request for a "take home assignment" is "bite me".

[@Fafix666]

After 13 years, it's my answer as well.

[@tiagovallejo]

I was already like that after 5 or 6 years! Waste of time and you are left at the mercy of whoever is reviewing the code likes your coding style or not

[@trader548]

I have seen companies asking for your links to your github projects and what your "score" is on stackoverflow. Like true professionals have time outside of working their asses off on proprietary systems for clients....

[@georgebelletty7861]

I get more white board tests than take home tests. I don't mind them, after serious time our CV's stand up for themselves I think.

Lack of empathy there... I review a lot of candidate assessments, and A LOT of them are utter sh*t, others copy/paste from github and lately just chatgpt garbage... you cannot just interview someone and know immediately their capabilities, and to test someone I prefer to give them time at home to do their best instead of a on-site test which just will make them nervous and pressured. On the other hand, yes, you are wasting 3 or 4 hours to do the assignment, but, from that logic, I'm losing as well time to review it... empathy... if you want something, you need to put some effort on it, don't you? Btw, I've been also working as developer for 30 years... that doesn't make you "the god of development" and expect that every company will just roll the red carpet for you... you are a total stranger for them, and you need to prove your worth.

[@stevepettifer4896]

Once had a code test rejected for putting the interfaces in the same file as the class on the basis that "It violated the single responsibility principle". Felt pretty OK about that.

[@BittermanAndy]

@@Robert-yw5ms I think the O, L, and D are much less widely understood than the S... to the point that I no longer think SOLID is very useful as an acronym.

@@BittermanAndy L at least has a very good/concrete definition (or a definition in the first place) - math baby

[@BittermanAndy]

It does, yes, but say "Liskov substitution" to 90% of developers (number made up) and they won't know what it is... even if they always do it correctly.

[@phizc]

​​@@BittermanAndy Just watched a video about that the other day. The ReadOnlyCollection kinda violates it. It implements ICollection and IList even though it's read only. Many of the implemented methods throws NotSupportedException. The reason for it is that the class is from .NET 2.0, while the IReadOnlyCollection and IRaadOnlyList interfaces weren't added until .NET 4.5. A week ago I would have been Lisky-who? But then again, these days OOP and inheritance is out, and composition is in, from what I can tell.

[@nunofigueira8691]

SOLID principal are not standard it is something a guy implemented. It don't improve your cose performance or reduce the CPU power consuption.

[@minnesotasteve]

If you give me this task, then you need to give me a sample of your code base. I need to see what kind of mess you are asking me to come in to support.

[@lmoelleb]

In my last two job interviews I looked at source code before accepting the offer. The first in their office sitting with a dev and looking at his screen as he walked me though the major components and as I asked questions zoomed into the code base where needed. The second I got a dump of a few of their repos. In both cases it was offered without I had to ask - in the last case even before I talked to anyone but the headhunter. Team lead positions where I was specifically brought in to work with existing teams where the projects had grown above their experience level. So they knew it was important I did not leave after 3 months because I did not like the legacy code.

[@minnesotasteve]

@@lmoelleb That would be nice. I have interviews where they ask about unit testing and other concepts and then find out it’s a monolith converted from vb6.

[@DGronki]

I wish my coworkers in banking sector be able to write at least code like that

[@jmbrjmbr2397]

Me too. We have methods that are thousands of lines

[@jag497]

I have never worked at a company that stays up on best practices. Hell I would passed that guy just on the principle you CAN write unit tests.

[@magnusm4]

@@jmbrjmbr2397 Geez. Writing everything in one class is bad practice. But one method?! Is it all just if statements?

[@jmbrjmbr2397]

@@magnusm4 yeah, anything you can imagine. Indentation goes brrrrr

[@Biker322]

I have hired lots of software developers, technical is usually a chat about coding , can pick up pretty quick if they know what they are talking about. I have never asked for a complete project done in the devs own time. I have rarely had a problem with the people hired that couldn’t be trained out of them. I’m looking for adaptability and the ability to learn and find out. If I was asked to do that test I’d walk. Can’t stand senior devs who laud their skills over other people. I see it as a problem in our industry.

[@BTimelessC]

I 've failed with way better projects and I 've succeeded with worse in interviews. It's a jungle out there and luck plays a huge role let's be honest.

[@jmbrjmbr2397]

I think so :( Interviewers want someone exactly like them

[@eminjungle]

Most of them are just opinions, or "how I would do it right now", and everyone thinks his fart smells the best. The reviewer is a moron, with low experience in professional development.

[@a.m.4154]

Underrated comment.

[@Code_Bits]

To be honest, this would only be okay for a junior position. Because if not, I should've been senior when I started working as a SWE.

[@Runningalien]

Watched this with interest as I provided feedback to the guy on reddit as well. And I'm happy to see you voiced most of the things I've said and a few others good points. I've also mentioned as well that 2 things are missing: exact requirements for the assignment and also what position level was he applying for... very important! Also, at my previous 2 jobs we had technical assignments - not simple, not rocket science, but designed with multiple possible approaches in mind. We ALWAYS had a 3rd interview in which we would go over the solution with the candidate and have a constructive discussion about the choices he made, what he would improve or add given more time. We were not interested in people finishing the assignment, but trying to understand how they approach a problem. Plus, we had people who submitted amazing solution, but they couldn't explain any of it as most likely they were helped by somebody else...

[@BittermanAndy]

Yeah. First-stage phone screen, second stage take-home test, final interview which (among other things) discusses the take-home test. Pretty standard.

[@Runningalien]

@@BittermanAndy I like this approach and found it to work well - sure, it takes time and commitment on both sides. But, you'd be surprised to find out that it's not standard after all...

[@BittermanAndy]

@@Runningalien I can't speak for every company, of course. But all the ones I've worked at, and almost every one I've interviewed at (successfully or not), have done that or minor variations. I can remember very few places I interviewed that did something different.

[@Downicon3]

Why the hell would fluent assertions make any difference compared to built in xunit assertions? To "impress", about what exactly? Adding extra dependencies just because... is there a specific reasoning?

[@nickchapsas]

It leads to way more readable assertions

[@Grumpicles]

​@@nickchapsas I find "more readable" to be highly subjective for every individual developer based on a lot of factors (e.g. experience, language backgrounds, familiarity with styles... brain chemistry) and tend not to consider that as a good reason in most development discussions. I think "what reduces comprehension" works better as a focus point because when reading code "I don't understand this" is what causes problems. Sorry, that went off on a tangent.

[@tajkris]

​@@nickchapsas How exactly code like ` value.Should.BeEqual(3) ` is more readable than `Assert.AreEqual(3, value) ` ? Chaining multiple assertions into one 'sentence' can be seen as less readable too - it' easier to read short sentences than long ones, isn't it? It's fine to have a preference for a library or a style but judging candidates on that is a bad.

[@petewarner1077]

Fluent assertions don't reduce cognitive load. They're a style choice, nothing more or less. We saw the same with nUnit and the Assert.That(value, condition) style assertions that everyone claimed were "more natural". It's horse shit. If you don't know what Assert.Equal, Assert.NotNull, Assert.Same mean, go read the docs. Anyway I'm thinking of creating an assertions library called Verbose, where instead of Assert.Equal(expected, actual) you do Assert.Politely(That.TheValue.Of(3), Is.Indeed.EqualTo(3).Thankyou())

[@BonsaiBurner]

@@nickchapsas That sounds like an opinion?

[@TheOnlyDominik]

The project is called "TripBookingApi", so the technical names of the directories are OK. I always recommend using business names. I don't use an "Interfaces" folder either, but why Red Flag? You are claiming something without explaining it. But I understand that you want to sell your workshops.

[@tacosontitan]

He explained it in a very rushed and subtle way by saying he'd group interfaces with their default implementations.

[@sealsharp]

Putting two types in one file seem more like a personal preference than an interfaces folder imo.

[@sf-petru]

@@sealsharp I find that very strange. Putting the interface inside the class file means you are using the interface with just a single class, and I don't see the purpose of that interface

[@sealsharp]

@@sf-petru Yes!

[@_tonypacheco]

​@@sf-petru I've seen this when the interface doesn't exist to enable multiple different implementations, but just to allow for mocking in unit tests

[@fifty-plus]

I've been coding for decades now and the only test I give is one where I watch them complete it. For the most part, I don't even care about the solution I'm analysing how they think and approach a problem and their attitude to doing so. I wouldn't expect them to follow any company coding standard in one of these tests, it's purely to see how they think and analyse the problem.

[@davidbaker251]

Somewhere a recruiting manager is cursing the fact they now need to make up a whole new coding challenge.

[@weicco]

After 25 years as professional developer, designer, whatever, if you give me home assignment that interview ends there. I do not work for free.

[@karlisbroders]

What if its paid regardless of being hired?

[@adsadam1]

@@karlisbroders As Nick says in his video, in the UK it has literally never happened to me and he's never seen that either.

[@weicco]

Maybe I should clarify a bit. In this video there was some valid arguments like problems with nullability. But there was many of those "I don't like this", or naming is wrong, or any other such minor detail, which is exactly why I don't do home assignments! It is demeaning that some 20 years younger (this is not directed against Nick) interviewer marks those as significant problems when they are in fact just differences in opinion. I've been interviewing people from time to time and I've never turned back applicant because of little issues. If the applicant can develop decent code, it doesn't take long to teach him or her how those minor issues can be done better. And besides, almost every work place has these almost fascist coding guidelines which can be a real issue with even seasoned developers. What were the red flag for me? Attitude. Can the applicant work in a group. My view of a perfect work team is people who can and are willing to communicate freely. This way all the minor issues can be handled in constructively, in a good manner, between everyone in the team. I hate the one-on-one discussions because every issue should be a team issue. Managers tend to patter "there is no I in a team" but they don't really understand what it means. It means exactly what I just wrote - all for one, one for all freely and willingly. If you like to find out more I charge 150 € per hour, 1 hour minimum 😁

[@draloalo]

Okay. Good for you that you can land jobs without actually showing your skills in praxis. I personally prefer home assignments. I have been to many interviews over the years. But the two times where a home assignment was required, were the two times I actually got hired in my career. So before you bash on home assignments, I will let you know that we are some people who are really bad at selling ourselves in interviews, but that doesn't mean that we are bad programmers. We are just better at doing the job than we are talking about it.

[@adsadam1]

@@draloalo nobody is saying they're bad, I got my second job doing a take home task. They should be paid to do, is all anyone is saying. These tests are usually 2 hours minimum, it's a joke if you have any sort of life outside of your current job.

[@benbaran4517]

If I was doing this for free I would probably make some of these mistakes because I would be going fast. But if I was the interviewer I’d definitely cut some slack and say “you have the general idea, but what could you have done better here?”. Work goes both ways in an interview.

[@collapsingspace]

What I see is that this applicant can most certainly write working code, code that does its job and code that you can read and modify later. In all honesty, one should move past to testing applicant's other aspects that would make him fit in the role. How good is his communication? Can he explain clearly what he has written? etc etc. Folder structure, naming conventions, use of older libraries and API's are all issues that can be sorted out in one sitting.

[@asagiai4965]

Thought process?

[@nunofigueira8691]

It is make part of the work agreement of the team.

[@da3dsoul]

I'm a senior dev, and I learned stuff here

[@Erik_The_Viking]

This is why I no longer do programming assignments for interviews. I've had several take home assignments get ripped to shreds for no reason other than for reviewers to fluff up their egos. People have said everything from "this is code smell" (because they didn't like my implementation without giving nay reasons why), "this code wasn't complex enough" (how complex do you want it? maintainability is a concern), among other colorful remarks

[@KonradGaska]

Just out of curiosity - why would you suggest doing _context.Update(trip)? Working with detached entities I'm considering as a bad practice in general. Update is traversing entire object hierarchy and its generating updates for columns which haven't changed, etc. Instead I would get attached entity from the db context, update its properties and called SaveChangesAsync.

[@sevenver]

I would probably use the efcore7 ExecuteUpdateAsync but i aggree with you

[@fusedqyou]

It was explained in the video, no? Especially when it comes to beginners these negligible performance differences are not something to worry about. When you write an application like this it is important that it is clear that you understand how to handle the scenario's that are required, including using newer features to tackle them.

[@loganyoung228]

@@fusedqyou understanding newer features and how to tackle them is precisely what I watch Nick's videos for!

[@_iPilot]

IMO, "Update" is initially implemented incorrectly because it consumes a Trip as an argument. Such an implementation cannot guarantee the Trip is obtained and saved by the same repository. It would be better to load, make changes and save the entity withing the only method, accepting id of the entity and new values for properties allowed to be updated as the method arguments.

[@KonradGaska]

@@_iPilot But Update method is designed to use for detached entities. In that case context will check if entity exists with provided ID and update it including upserting all the children entities, etc. Trip wasn't obtained from repository, but posted as an input through API. Keeping both models the same (request input, entity model) is also a very bad practise

[@CRBarchager]

Very nice walkthrough of the code. 7:30 I missed the problem here. Was it the naming of the controllers or is the prural-part that's the problem. You mention is should be lowercase but what should you write to make that happen?

[@loganyoung228]

You wouldn't rely on [controller] in the Route attribute to control the naming. That opens you up to a fair amount of guesswork because you can't be sure what the endpoint would be. Instead make it explicit: [Route("api/trips")] or [Route("api/registrations")] rather than [Route("api/[controller]")] ... using [controller] is just... lazy.

[@CRBarchager]

@@loganyoung228 Thank you for clearing that up. I've skimmed though some of our codebase and we have this exact naminig issue. Didn't think much of it until now but most of the codebase is not comprised of API controllers though we are heavily heading in that direction so removing this mistake now is great.

[@jcx-200]

@@loganyoung228 Just curious to see if you would hold the same view point in regard to implicitly typed variables as being lazy or if you would have an exception for that.

[@BeekuBird]

​@@loganyoung228 It's an MS convention. Failing someone for following an MS convention when the interviewer didn't specify standards is not correct.

[@billy65bob]

These APIs are all case insensitive anyway, so I don't see the problem myself.

[@Ayymoss]

This code is definitely for a junior position.

[@ChristianMartins]

Committing artifacts was the biggest red flag for me. The reviewer seemed to think OP was able to read his mind

[@davideastman5806]

So many of these things are opinions and code style things that could be corrected in the first week of the person's new job. A hire is meant to be at the position for at least a couple of years during which time all of these things could be correctable. I understand a fail for a senior who needs to work on a project immediately with no supervision, but for most every other case, as long as this person is amenable to changing their style, they could be an asset. Not to mention that they're publicly seeking feedback for their work, which is a green flag.

[@TheOnlyDominik]

I have never been asked this level of question in a job interview. Software development should not be seen as a science. If I were to develop at this senior level, nobody could/would want to pay for it. It is important that certain specifications are made, but these are only developed in the project and are constantly changing. For example, I didn't even know the "return Problem();" and I've been developing software for over 30 years. The most important thing is that the software works, is secure and performs well. How long has such an app/service been in use, so there's no need to over-engineer anything?

[@MarkCastle]

Yeah, I hire people based on factors such as “can they solve difficult problems” and “will they fit well into the team”, more than just “are they a total robot with 100% best practice knowledge only”

[@Runningalien]

@@MarkCastle Amen!

[@_iPilot]

@@MarkCastle unfortunately, for many projects keeping solution structured, consistent and with predictably located components is an extremely difficult problem.

[@Runningalien]

@@_iPilot I don't think I've worked in 2 places having the same project structure, coding standards etc - sure, there's overlap, but never full agreement - look online at any coding debate and grab popcorn. You can learn new "best practices" and the new team's overall naming/coding standards easier than you learn how to solve difficult problems (you can learn that as well obviously, but takes way longer - that's experience). Also, much easier to "police" naming conventions, project structure etc than plain poor solutions for a problem.

[@davestorm6718]

@@pyce. I disagree. Programming is not science at all. It's technical practice where standards change all the time and preferences differ everywhere. MS's best practices are not always in alignment with the current "best practices" of the day. Look at the scaffolding with the [controller] convention - this is MS's convention (actually a particular convention of the writers of the .Net MVC design pattern which somewhat resembles the MVC design pattern from the early 1970s). There is a lot of creativity in the programming field and it's absolutely necessary. There is no one-size-fits-all. With that attitude, you certainly won't be moving forward in the field. Programming (like other practices, including medicine) is never truly mastered - the industry changes so rapidly. If you're unable or unwilling to adapt to these changes (requires creativity, problem solving, flexibility, etc), you'll be left behind. That monolithic mindset guarantees future failure.

[@adamlindqvist2570]

It would be interesting to know what position this was for and the time frame. Because even though I agree on some points, failing someone for standards feels kind of harsh. Also if this was a 24 hour type of test, the interviewer should have gone through the solution with him like you said, maybe some decisions was made based on time? I was expecting a train wreck solution but this code is not that bad if I'm being honest. The only thing was the tests that could have been better.

[@BittermanAndy]

As someone mentioned in another comment: going through a solution with a candidate that a company has decided doesn't meet their expectations just isn't practical. My company is pretty ordinary (not Tesla or Microsoft or Google or any of the big ones that people get excited about), and for any open position we'll typically get through 75 CVs, 50 phone screens, 25 coding tests, and 5-10 final interviews before we fill the position. We can't afford to give 15-20 people who didn't pass the coding test detailed feedback or a chance to justify themselves, there just isn't time, we've got work to do (and 5-10 better candidates to take to the final stage). If they're actually good and we judged them unfairly, they'll find somewhere else to work pretty quickly.

[@adamlindqvist2570]

@@BittermanAndy I understand your point, and I somewhat agree, but at the same time i think that if a candidate is expected to put in this kind of work for a home assignment this should not be the feedback they should expect. Now to be clear, there are a lot of factors missing here and like position, what was the instructions etc. but based on what I see in this video he did what he was asked to do. So getting this vague and personal feedback after such a test does not look good in my eyes.

[@BittermanAndy]

@@adamlindqvist2570 that's fair, and I think the actual feedback he was given was quite rude, and worse than no feedback at all - I'd have just said "no thank you" rather than say "this is terrible". But I'd stop at "no thank you".

[@tbddevelops]

I've given a coding sample test before, but with no expectation on perfect structure or even usability, I use it only as a discussion to see if the developer and I can discuss concepts and principals with a common context. If I'm hiring a junior-mid developer, I'm ok if their practices don't align to mine, those are things we can discuss and ultimately, can the developer follow standards the business lays out is more important to me.

[@melnor82]

If you don't like the route [api/controller] attribute, what's your preferred way?

[@lekretka]

I guess writing [api/trips] manually.

[@nickchapsas]

Building an ApiRoutes class and separating the endpoints by sub classes and computed routes

[@loganyoung228]

@@nickchapsas I'm gonna need a video on that, if you can manage it! Sounds a bit scary. Right now I'm building my APIs more or less based on MapIdentityApi() in Identity which probably also isn't ideal... but for the first time my API works at least!

[@BTimelessC]

@@loganyoung228 It shouldn't sound scary. You 're basically using constants in place of magic strings so you can ensure conformity. Think of it something like this : internal static class ApiEndpointsConstants { private const string ApiBase = "api"; public static class Product { private const string Base = $"{ApiBase}/products"; public const string GetAll = Base; public const string Get = $"{Base}/{{id}}"; public const string GetPrices = $"{Base}/{{id}}/prices"; public const string GetHighestPrice = $"{GetPrices}/highest"; public const string GetPriceRecommendations = $"{GetPrices}/recommendations"; public const string Update = $"{Base}/{{id}}"; public const string UpdatePrices = $"{Base}/prices"; } public static class ProductGroup { private const string Base = $"{ApiBase}/products/groups"; public const string GetAll = Base; public const string GetCompetitors = $"{Base}/{{groupId}}/competitors"; } } Then you just add the route tag and pass in the constant (e.g ApiEndpointsConstants.Product.GetAll).. You can even manage versioning like this way easier. In the end, it's just a string, but you don't let your human errors make typos (if you make a typo, it goes everywhere basically)

[@dosovi4548bustayes]

@@nickchapsas more files and folders..

[@Mr__B.]

I think this format of videos will be very valuable for people who is seeking for a job at the moment. I've started my new senior position, but it was harder than usual to find a job recent months because of economical recession last 12-16 months and such overviews from prospective of employer will be super-worthy! Thank you for this video. it is easy to make such mistakes even with experience when you are focused on business logic you're trying to implement.

[@roelwestrik2956]

It's crazy how you can fail someone over something so opiniated. Sorry Nick but I strongly disagree with your points too. IMO an interfaces folder is fine. Storing interfaces in the same file as their class??? Ew.

[@jeroen7362]

its not that ew, the interface and the implementation are so strongly coupled that they are practically married. Its very handy to keep them close in one file with one concern, you will never change the interface without changing the class. And be not strict about it, if you have multiple implementations the interface deserves its own file imo.

[@PyronTheMage]

Brilliant insights. I'd be interested in seeing you do this test and share your solution and explain your decisions!

[@iSoldat]

I've had these types of "tests". They aren't looking for good coders or coders who think outside the box at most of these places, they are looking for a clone of themselves. They are stuck in the past and have mantras like, "if it ain't broke, don't fix it." When you fail those interviews, it's a good thing.

[@cloudenvier2260]

Love that kind of content! I'm currently working for a company on older frameworks and having a quick run through of the newer practices like that is pretty insightful.

[@JerryPlankinton]

26 Year Software Engineer here... I have been hiring devs for the last 10 years. The artifacts in the repo is a deal breaker for me.

[@paulmdevenney]

Vertical slices are great, but MVC doesn't lead you that way, particularly with views. A lot of what you describe are nice standardisations, but some of these are at best your personal preferences. Not of it identifies whether this person can analyse problems well and solve them. This person might have specifically been taught a different style. If I saw constant newing of http clients, or not using DI, I'd be worried. But I'm not sure whether this level of picking at the code actually results in a better hire.

[@alexbarac]

Always have a 2nd talk after such an assignment, there's things to learn on both sides. Nitpicking on details is wrong imo, those can be discussed in the 2nd talk. Just the bigger issues are worth considering and be mentioned directly. 13 years in software development taught me that there is valid reason (in the authors pov) for the written code and that it's worth having him explain.

[@Ch17638]

In an interview, I once did an assessment; even though the assessment stated I could change the project in any way I saw fit, I got blasted in the review for not following "very explicit guidelines" after I pointed out there were no guidelines on the instructions their lead dev said that even so, I should know best practice. So when I cited best practices in each instance, he gave me and included the recruiter on my side. It went silent for a week, and then someone else in the company mailed me and asked me to come in for the next round of interviews; I declined , the small taste I got what it would be like to work for a company like that was enough to chase me away.

[@zbaktube]

About NewtonSoft Json: I add it for parsing enums + document it well with swagger. My memories not so clear on it, I did it a while and I am just reusing it like good functioning robot.

[@viniciusmelquiades]

Last time I used C# Newtonsoft was still the standard of JSON serialisation, and I'd probably use it for this kind of test if I had to get back into C# development without much time to study the newer stuff

[@aceoft3482]

While there are some legit complaints that I agree with here, it seems we need to add "look up the interviewer's personal preferences" to our list of interview prerequisites to avoid petty judgements. With the speed and frequency of the changes to what is "right" in dev, and the fact that there really isn't any single "right" way to do so many of these things, I think you both are missing the bigger picture. These exercises are supposed to be designed to learn how someone solves problems. Not if they named the folder what you would name it, or even that they solved it in the exact way that you would. It takes some time to adapt to a team's standards, which are, btw, different everywhere.

[@phizc]

One interesting possibility is to have a demo project, and let the interviewee reference it as a style guide. That way the exercise would also show how good they are to adapt to coding styles, and they would have examples of what the organization is looking for in unit tests, project structure, and so on. E.g. should they use FluentAssertions or not, multiple projects or not, etc.

[@leriosindane720]

Great Video It’s great to continue making these types of videos. They make us reflect both as developers and as managers.

[@julianocs87]

11:08 what should be done in this case? Why is it "wrong" to return a message when somethng is not found?

[@tbgelectr0]

My guess is that the httpstatus 404 means not found.. and it should be enough. But I agree I would not have a issue with that

[@dabest9843]

So does Nick have a version of this code on github that is the "correct" way?

[@Wfmike]

Let's be honest people will still find holes. API design is quite opinionated

[@codefoxtrot]

As a senior level developer, I do take the time, maybe 2-3 times per year to write little challenges like this. I feel it helps keep me fresh to build something from the ground, even if it's a simple CRUD app-- it can be strangely satisfying to know you can build something 'off the cuff'. Ask yourself when the last time you just impromptu built a small challenge, even a simple CRUD app that you makeup yourself. Sometimes it surprises you what you forget, and just as often, how far you've come!

[@AndrewRussell-w4l]

Nick as you pointed out context is key here. Whenever I review an assessment, I am well aware that the candidate has put in their time and effort to creating a solution, even if it doesn't meet the brief. With this in mind, I always supply a pros and cons list back to the candidate so they can at least get some value out of the exercise.

[@kevinlloyd9507]

I'd like to see the full "take home" instructions. Because if there was no clear context, a LOT of this feedback is subjective. Been doing .NET for almost 20 years and I've encountered teams writing code like this all the time. At my current job, we've already recently agreed that Vertical Slices are preferred. We've had "Interfaces/Models/Controllers" folders for years. Everything needs context. And maybe I'm alone in this, but I kinda like take home interview assignments. LOL.

[@jcx-200]

I would say a great deal of this criticsm is inherently subjective especially around the namings/folder structure. Could be a red line for one company, but could fit the exact sturcture desired for another. Realistically you aren't going to please someone with every single aspect to the maximum degree. If you want them to follow it to a tee, then surely you would provide that sort of information in a brief (to a degree, I understand wanting to see them use some of their own thinking behind it)

[@BermudaLamb]

Nick, you mentioned in this video about using "[controller]". For our api's we have a sluggify manager enabled that sluggifies our controller and endpoint names. However, in many cases we do hand code the endpoint name for more convenient readability.

[@mtsmithtube]

@9:57 you removed the return statement?

[@mtsmithtube]

Oh nvm. You went so fast on the video its hard to see the change. Slow down :)

[@Folderq]

Red flagging for opinionated tools and actions is already reviewer disability. Code should be good and up to the standards. Failing people, just because I would do something different way, is unprofessional and it's actualy a red flag for joinging such team - stay away! At least they should have deeper conversation about reasons standing behind choices, instead of dumping him.

[@FlorianDelorean]

That is an unexpected reddit user name for this kind of reddit post... But then again, maybe not 🤣

[@asagiai4965]

Lol nice name

[@TrueCodePoet]

The key factor is whether the project meets the requirements. If you have specific standards you hold developers to, communicate them clearly. There's nothing wrong with informing candidates about the criteria you'll use for evaluation. The primary objective is to determine if they can complete the task and follow instructions. If the requirements are ambiguous, it reflects more on the requester than on the developer. Consider this a positive outcome. If you didn’t get the job, it's likely you wouldn’t have enjoyed it. For those new to the field, I recommend interviewing as much as possible. Remember, getting a job and doing it are two different skills, and both require mastery.

[@molokhai]

Here is my opinion: - I would not use try catch in any of the methods. This is a very simple api. He could have used minimal api. - NotFound should be trigger in the service layer. Testing for null in the controllers is just not clean. Nullable reference type is something you use explicitly, because returning null was abused in the passed. If the method is suposed to return an entity it should return the entity and throw an exception if it cannot. - Catching DbUpdateConcurrencyException? Why would you catch this exception? Let it throw. Exception are meant to terminate the api request indicating the request could not be handled. - Logging? i havent looked into the code but every exception should be logged. - The update and add methods in the service layer should return the entity. There is really no reason not to return the entity. - Tests: Unittesting, integration testing and mocking framework? Where are they?

[@mome3807]

If the purpose was to demonstrate a production ready application aka. many devs could continue working on (maintainability is key imo), then there should be more middleware present for errorhandling, authentication, resilience etc. and then all logic placed in services. Placing it in one project is ok, but I’d prefer feature folders in this case where models, services, interface etc. for Trip is in a Trip folder, but that might be opinionated- but it makes it much easier to maintain especially when the application grow.

[@ThekillingGoku]

Folder structure is a generally subjective thing. I've been doing this stuff for close to 2 decades now and I have absolutely no problem with the MVC-style folders (AKA 'Controllers'/'Models') or even having seperate files for interfaces. But in general, that'soften a project or even company level 'agreement' or decision on how they like it. And no, I never stick my interfaces in the same file as the classes. I do generally NOT stick them in a seperate namespace though, since that's kind of annoying on top of offering me no benefits, only demerits as mentioned. Anyway, for a simple sample project the classes directly in models is fine. But if the project were to grow, I'd still have a models folder, but with some extra structure inside like folders/namespaces for all regarding registrations and one for trips. And if it got even larger than that, I'd start dividing up into 'Areas' at the top level. Yes, once again, MVC style. But there ain't nothing wrong with that. As for the single project shizzle, yeah ... there ain't no problem here. The scope on this thing is nowhere near where you'd need to start splitting this up into multiple projects, so that'd only be unnecessary overhead here. Failing an applicant for that is kind of ridiculous as that's like failing you for not killing a fly with a sledge hammer.

[@jakebounds5527]

Genuine question: When would you need to use multiple projects for this? It looks like one project that does one thing, which is handle trips.

[@billy65bob]

The current fad is kind of to have something like 1. One per domain to define annotated models, 2. One to handle, initialise, and maintain the Database, and pull in the annotated Models 3. Another to define its services 4. Yet another to define its controllers per domain 5. One to glue it together as a Executable 6. One for each of the above to do Unit Tests. It's not really my style; most I'd personally so is split the models out so I can easily make a user facing library for interacting with my API as intended.

[@FarmerAstronaut]

@@billy65bob And it is always funny to look at projects with models while all those models are anemic.

[@Shynobyn]

Regarding the "All in one" thing I remember the opposite happened to me and the recruiter was like "why are you using multiple projects?" Although he gave me the opportunity to explain and didn't fail me for that despite my answer wasn't good.

[@TrOgaN_]

When you are a professional frelancer you will come across a lot of code bases that are out of date, badly implemented and not using the latest dasign patterns. You have to accept this if you want to pay the bills, and make the customer happy by making the changes you are asked to make. At the end of the project you could suggest that some of the code requires updating but most companies will ignore you as they probably have other priorities.

[@xlerb2286]

Yeah, if I were the reviewer I wouldn't worry about the little things like all code in one project, not using fluent asserts (Imo fluent asserts are many times a solution looking for a problem). That could just be time crunch, nerves, or trying to make it simple for the reviewer. Style issues can be taught, I don't care about those. Things that show unfamiliarity with development in general are more worrying. But even there depending on the position being applied for it wouldn't necessarily be a fail. I was more interested in whether the person "thought like a developer" than whether they were familiar with any specific framework or technology. In this case the biggest worries I saw were things like checking whether 'registration' was null and if so returning null else returning 'registration'. That is not thinking like a developer and by itself would likely have been a fail for me. And fair disclosure, I likely would have failed that test. I've worked with C# since before it was released and was known as COOL and I've never done a web service other than back in the WCF days so I'd be sitting there scratching my head and would likely have made some bonehead mistakes and not got the job either.

[@georgebelletty7861]

The decision was correct in my opinion, as the developer is using Newtonsoft I'm guessing they are not a junior. If I was the interviewer I would have asked the interviewee to use multiple projects akin to a clean arch. design. BUT... the developer did post on Reddit so they are very keen to improve which is a major plus point in my opinion. Hope they see Nicks video and learn and practise. Good luck to them.

[@phizc]

On the other hand, if they're using NewtonSoft, wouldn't that mean their experience predate STJ? 🤔

[@asagiai4965]

Wait, what? Is there any relevance of using Newtonsoft in any of this?

[@DavidFernández-b7s]

Naming a directory "Interfaces" a MASSIVE red flag?? You "standardization paladins" are the biggest red flag. Then I ask you to use the command line, deep understanding on Git or to solve dynamic linking library issues and start crying because you thought you were the best engineers on Earth.

[@nickchapsas]

Do you have a folder called Classes and one called Structs?

[@DavidFernández-b7s]

@@nickchapsas No, Nick, and I partially agree with you but I think that just saying something like: "I would name directories based on what they do and not what they are" would be more than enough. IMHO a take-home assignment should be reviewed based on exception handling, SOLID principles, testing and not much more. Judging people calling them "massive red flag" because they are not using standard principles is a big mistake.

[@Fafix666]

A lot of complaining over nothing here, Nick. Plural names in endpoints? Who cares? It changes nothing. It doesn't make anything more or less maintainable, extendable or performant. It's an empty standard that means nothing and not following this is far from showing that somebody doesn't understand REST APIs. It's a smell at best. Interfaces with classes? That's a smell, mate. What if it's a strategy? Will it be inconsistent with the rest or how do you plan on solving this? How about we also put DTOs created by that class in there? Reviewers can prefer Minimal APIs or Controllers, but if they fail you for using something else, you don't want to work there. It's a toxic place based around tech leads and architects licking their own balls. Definitely not a senior-level code, but easily above junior.

[@hck1bloodday]

I think he mean interfaces that have a single implementation should be in the same file, because you can change the class and the interface without having to navigate between files (that can be a pain in the arse in some architectures)

[@Fafix666]

@@hck1bloodday I know exactly what Nick meant. Still not cohesive.

[@Cornelis1983]

I also put interfaces in their own folder and hate when people don't. It is so annoying when you scroll to a list of files with classes searching for a class starting with an I and you have to search for a needle in a haystack because all those files with interfaces are in that folder as well.

[@anonanon195]

13:31 LMAAAAAAAAAAAAAAAAAAOOOOOOOOOOOOOOOO THE NICKNAME

[@iphill4813]

More like a guess reviewer preference game than a skill test. And fashion is often chanes. All in all, if it's not a green field project job, a newcomer will see all architecutal decisions in fn a couple of weeks of real work./

[@Sergio_Loureiro]

As a person, who unfortunately still has to code on my job using the old .Net framework, I feel this person has an old .Net framework mindset, instead of new .Net Core thinking way. And it is pervasive on all the Internet the old way of doing things, which is where this person may had his/her inspiration coming from.

[@FarmerAstronaut]

Could you please elaborate on that? What gave you that clue?

[@ashundeyan8031]

As someone who has been responsible for hires before, I think this person would be fine for an entry level role, but I would seriously hesitate to hire them for anything above that. I think they'd be able to catch on in a few months working with an actual codebase. And, I agree with Nick about the "all in one" thing

[@vincentotieno9197]

An all-in-one project for a production API would be a bad idea. The candidate needs to demonstrate some understanding of at least one architectural pattern. Other developers might need to maintain the code long after the original author leaves the company.

[@BeekuBird]

@@vincentotieno9197 The popularity of N-Tier with anemic domain on this page only demonstrates low quality of the industry generally.

[@Wfmike]

As a senior cloud architect I'd say to the recruiter focus more on the attitude while hiring than coding tests.

[@nilscoussement]

What utter bullshit to fail someone for checking in unit test artifacts How long does it take to learn to code like that? 5 years? How long does it take to teach a programmer to ignore a folder? 50 seconds? I think I made my point, how toxic!

[@hck1bloodday]

also if you create the repo in github directly you can select your gitignore (visual studio is one of the options), so yeah, just tell him to select that and you are good o go, 10 seconds at best

[@oliverhall2387]

He did say that it explicitly said to not check in the artifacts, so I think it's a failure of not following instructions.

[@nilscoussement]

​@@oliverhall2387 if he was a doctor, you would be right. Just send him a link to something that explains how to do it, and ask to resubmit. Might be oversight, might be that he did not know. Heck, might even be a time constraint as this was done during his 'free' time. If you expect everyone to always be 100% perfect, be ready for a big supprise! Why do you think we have peer reviews? I remind you to think of what is required of someone to do his work. And if the remaining imperfections in his work can be resolved.

[@Switchs89]

If you still haven't learned after 5 years not to check in the artifacts, then boy... there is no hope for you. It is a massive red flag, and it's not an opinion. It's almost equivalent to not understanding the purpose of a Source Code Control System like git. This is fundamental knowledge. You cannot screw up on things like that and still consider yourself a good software engineer.

[@bartlomiejuminski]

More code review videos please

[@loganyoung228]

Nick, I'd like to hear more about how you'd structure it. You said using verticals? I understand you'd have everything to do with your Registrations and Trips all foldered together which makes sense, but then you mentioned adding the interfaces to the class to simplify and I'm very curious about how that works.

[@docbrown2045]

I think he meant having class and interface in the same file? Which reduces amount of files.

[@duznt-xizt]

Vertical Slice Architecture has been a godsend for me in complex projects. Organizing by technical concerns is sooo last century 😎

[@davestorm6718]

@@docbrown2045 I always heard this is a terrible idea. Not sure why, actually, now that I think about it. hmmm

[@nickbarton3191]

I think it better that if possible candidates come with examples of their own code, and talk and discuss it. Better than a take home project. I ran some interviews recently, for embedded. I asked them to draw a rough component sketch for a simple English written requirement, should have been half a dozen components. What I was looking for is dependency inversion. Not one demonstrated that. So the fact that this guy had Interface(s) at all was a plus point.

[@logank.70]

Doing interviews is difficult. There is so much subjective opinion around development. Companies vary wildly over what good practices are and people within those companies (depending on how large it is) have different opinions on those same topics. As the person being interviewed it is impossible to know everything that the interviewer(s) like and to code in that way. There is also the situation where somebody would be considered a senior dev but they've only worked for the one company so they aren't very experienced. I've seen that many times where they had never heard of NUnit or Xunit only because the only company they ever worked for (for ~10 years) had never written automated tests. Same goes for other technologies. In the past I've tried to take as many things into consideration as I can. If it is for a junior or mid-level I will use the personality tests to figure out if the person is malleable or not and look at their code exercise from that lens. If it's somebody senior then I'll take their resume into account to get an idea if they are experiences versus just tenured. I'll also keep track of all the things I didn't like and when we go over it we'll talk about them. With some coding exercises I've written the test suite myself and gave them the interfaces to implement. That way I can do the whole "if any tests fail we don't move forward" because I knew what the test suite was and that it covered just about everything. tldr; Conducting interviews is hard and it can be a little unfair to blindly say `no` to someone on things I don't like because of how different the industry is about almost everything.

[@BittermanAndy]

Yes, there is a lot of truth to what you say here. OTOH I wonder about those people with 10+ years experience who've never heard of NUnit/XUnit/whatever - I've encountered quite a few myu self. Do they not watch KZbin videos, read blogs, or otherwise keep up to date with the state of the art? I've interviewed people before who've said "oh, my current employer doesn't do X so I don't know anything about it", and I've thought to myself, "well, other candidates have got X, so what are you going to do? It's your career, are you going to learn it in your own time or moan about how your current employer won't teach you?". There's limits to that of course - expecting a candidate to know every last detail of a very specific obscure API would be absurd - but, like, unit testing as a general concept isn't new or controversial any more, and hasn't been for a decade or more. People need to know fundamental concepts, and if their current employer won't do it and they haven't bothered to learn it on their own time, they're not sounding like a great hire.

[@IElial]

Ha.. I create lots of Interfaces folder throughout my projects ! Usually I have a main "Sources" folder and under several "sub domain" folders (usually again "Utilities", "Business", "Factories", "Exceptions", "Serialization", etc..) (could be repeated into more deepness if needed) and down this folder hierarchy, I end up with one "Interfaces" folder having a file for each of my current (sub)+domain interfaces, and all my implementations of this (sub)+domain interfaces. Am I that wrong ? And I don't understand how you can merge your interfaces code in your implementation class file since you could end up with several implementing classes right ? (or maybe it is something around Rest-API / mini API do not have several implementing classes anyway ? I don't know web programming)

[@toneylol]

Recently had ~6h interviews and tasks for a position. Recieved a "no" after the fact, with no feedback at all 👍

[@Runningalien]

then consider yourself lucky - if you spent 6h interviewing and they didn't even bother to provide feedback I think you dodged a bullet :)

[@theonlywallrus]

This is good senior level code. Nick's comments are spot on, but I would be more lenient. Most of what he said can be fixed in PRs, and updates after deployment.

[@frankroos1167]

I rarely go flat out "Oh, that is wrong". I'm more of a "Can you explain that?" type. So unless it was for a job where the applicant had to take the lead and show others the way, I would have asked for explanation. Also, asking for explanation gives an opportunity to find out more about what an applicant can really do. In particular when it comes to considering alternatives and learning new alternatives. I find those important in most jobs I have had. More so than knowing things already. The few times (didn't switch jobs too much) I had to do a test like this, they seemed to get more from the talk afterwards as well. So, at least where I live, there's more like me. My guess is, the real reason he didn't get the job is that there were better candidates, but they didn't want to give that away. And after seeing you review it, I do think the comments he got do give clues to what to improve. It may not have been totally horrible, the points were definitely where improvement can be made for a next time.

[@flibbertigibbet6324]

Recruitment is hard and good developers are in short supply. Employers who erect artificial hurdles to reject candidates damage their own future. Opinion is cheap and if enough developers pile into a code review, there will be an exponential explosion of dispute and a plunge in productivity. Even some of Nick's opinion is odd. For example suggesting that the update returns a count of updates instead of a bool is an minor example of premature requirement embellishment. I would mark that down in a code review because I would now worry some calling code needs to be updated.

[@BittermanAndy]

Recruitment is hard, but the hard bit is finding GOOD developers, not just any developer. That's why you have a test, to find the good ones. It's not an artificial hurdle, it's a quality bar.

[@flibbertigibbet6324]

@@BittermanAndy The important thing is to erect useful test hurdles and not reject a candidate for transgressing prevailing local coding culture.

[@davestorm6718]

why would you need a count for a single update? I would just want to know if it were successful or not, though, I suppose a count of 0 would indicate a fail, and a 1 a success, but I'd rather code the error in for logging (what failed?) and send a success yay or nay back to the caller. I've used negative numbers to indicate fails with a specific code before (works great when someone says "hey, I have a -876 code returned!" and I know it's a connection failure, for example). This probably does not follow the REST standard.

[@codefoxtrot]

When I interview candidates, and dependent on position (junior, mid, senior), I'm more concerned with the potential to grow that someone offers. Sure, there needs to be a base level competency too, but the ability to learn and grow outweighs that. It also depends on the product... are we rushing to market with something, so we are limited in time to bring someone up to speed, or do we need an older app maintained with small feature requests and bug fixes.

[@TehKarmalizer]

Amen to not checking in artifacts. Working in a codebase where a developer checked in a nuget packages cache with over 6k files in it. 🤮

[@rubenverster250]

I've literally reviewed the specs for THA, and responded with, "Your THA closely matches the AC of this project I've built, please review this repo"

[@a13w1]

Didn't know about that Problem object, also for these tests I do feel making multiple projects would be too much time and don't do more than one myself as yeah not making production code. But yeah I do hate it on these when you do something that works but not what the interviewer expected so instantly fails as not there way of doing it (but not mentioned that anywhere in the test requirements) and then they don't give good feedback as to why.

[@asar9646]

whats your opinion on adding the suffix Async on method names?

[@thejs2249]

Nah, many subjective takes from Nick; how come having a directory named "interfaces", besides personal preference, is a red flag? Does it hurt readability? Does it affect performance? Kind of pointless...

[@nickchapsas]

It does hurt readability. You would never group all the interfaces together in the same way you’d never group all classes or all structs together in a folder called Classes or Structs. Think of how people will look for things in code. No one will ever say "I wanna see all the interfaces of this codebase". They will say "I want to find the interface of some class"z

[@thejs2249]

@@nickchapsas I disagree and nor I think this is a massive red flag, because I think “interfaces” does convey a meaning, in terms of “contracts” defined for my application. But I appreciate the explanation; it’s better to give a reason of why you’re calling something a “massive red flag”, instead of letting someone wondering (specially beginners) why they’re doing “wrong”.

[@pmoneyish6869]

​@@nickchapsas To be fair there are like 10 different ways to find something in a solution. Going thru the project tree structure is pretty low on that list of how you'd search for something when you're working in the solution.

[@vilecage1]

I failed a test once which was a huge take home test, took me over 7 hours to complete, and the feedback was that I didn't use Automapper :)

[@bitmanagent67]

A lot of interview criticism is gatekeeping and pedantic behavior. Yes, the person should be able to code, but a lot of criticism is style cop BS. Each person, team, department, and company has coding and style conventions. Most of the time, the IDE or extensions will correct syntax or suggest naming. To think that a dev who codes to one convention that is different from yours is a failure is the reason that people have a distaste for the industry. Personally, I would not want to work with you. Does it work? Great! Fix the rest in code reviews and refactoring sessions. I actually have sympathy for the candidate who submitted this. How can he improve if he gets raked over the coals based on subjective nuance?

[@amirhalloul]

Is there a video or an example for how you would organize your code? Like what should be in a different project, what should I name my folders, when is it okay to have multiple classes in one file or an interface with its implementing class etc...

[@Grumpicles]

If you have a candidate strong enough that you're going to ask them to spend considerable amounts of their own time on a task, then the least you can do is give some time to sit down and work through the critiques in person. I started refusing take home tests 7 years ago when I'd spend insane amounts of time building up a solution, then get nothing more than a short nit-pick as in this case, or a long bullet point list of even more ridiculous nit-picks, usually from devs with less than half my experience. I was telling recruits to exclude me as a candidate if they wanted any sort of testing. It's as much an employer filter for me as a candidate filter for them.

[@dinov5347]

Sorry to nitpick your nitpick but ReST is not a standard. It is an architectural style. You can't validate it per se but consistency is the key. I could argument plural like /trips vs /trip if you want to handle singular entity responses vs list responses (ie /trips/{id1,id2} vs /trip/{id}. I tend to do plurals since this is common. Also, ReST can't have verbs either which is in my opinion is not correct. My advice is always the same: be consistent (as you pointed out) and use common approaches but not be hamstrung by it either (like verbs).

[@michielvanhimbeeck6206]

Hi Nick, can you do something like this more often? Or like code reviews? i think it would be of great help. Thanks

[@MsDingens]

Can someone explain to me whats wrong with the [controller] in the Route attribute? Is it because it's missing the "/[action]" or should it be handled somehow else? the .net 8 docs at least show that as an example on how it can be used.

[@pedramkavian]

Changing the class name=Changing the already exposed API urls.

[@theMagos]

@@pedramkavian If you rename it to "bananas" why should it still be exposed as "apples"? Makes no sense.

[@pedramkavian]

@@theMagos Well I can't explain it to you if you have never created apps based on microservices in backend that are served by mobile and different client applications.

[@BlueScreenISU]

using [controller] is just fine, I also disagree with Nick here.

[@cybercyber1528]

nothings wrong with it, if someone rename the controller they can simply just keep the old route by using ("api"/"OldName")