Application Coding Test of 3 days

Hello,

I would like to share with you a code test I did for a position of a Senior Unity developer in a medium size game development studio. I did the test because it's hard for candidates with a long experience have options. I don't like tests that can take more than 1 day, they are abusive, but sometimes you don't have any other option.

Since they ghosted me after I delivered the test I feel free to share it with you. Unfortunately I cannot tell you the name of the company, only all the details of the test itself. So if you happen to find a similar test yourself, you will be already prepared.

All the documents, code and binaries ready to play in Android, Oculus Quest and Pico Neo are available in the repository:

https://github.com/EstebanGameDevelopment/CodingTestXYZ

Enjoy.

 

Took a quick look through the code. A couple of constructive criticisms.

First thing that jumps out is that you gotta either use tabs or spaces, your indents are inconsistent and it makes the formatting look incredibly sloppy.



If I were hiring, as petty as it sounds, this alone would probably disqualify you.

Other thing I notice is the number of compiler conditionals is very high and super annoying:

Code (csharp):

1.  
2.    
3. #if ENABLE_NETWORKING
4.                 _hasStartedSession = false;
5.                 _registeredPlayers.Clear();          
6.                 _scorePlayers.Clear();
7. #endif
8.             }
9.             if (nameEvent.Equals(PlayerView.EventPlayerAppHasStarted))
10.             {
11.                 PlayerView player = (PlayerView)parameters[0];
12.                 if (_playerView == null)
13.                 {              
14. #if ENABLE_NETWORKING
15.                     if (MainController.Instance.NumberClients == 1)
16.                     {
17.                         _playerView = player;
18.                     }
19.                     else
20.                     {
21.                         if (player.NetworkGameIDView.AmOwner())
22.                         {
23.                             _playerView = player;
24.                         }
25.                     }
26. #else
27.                     _playerView = player;
28. #endif
29.                     if (_playerView != null)
30.                     {
31.                         _playerView.Initialize();
32.                         // Game Level View
33.                         if (_levelView == null)
34.                         {
35.                             _levelView = (Instantiate(GameLevel) as GameObject).GetComponent<LevelView>();
36.                         }
37.                         _levelView.transform.position = Vector3.zero;
38.                         _levelView.ReportInited();
39.                         SystemEventController.Instance.DispatchSystemEvent(EventMainControllerLocalPlayerViewAssigned);
40. #if ENABLE_OCULUS || ENABLE_OPENXR || ENABLE_ULTIMATEXR
41.                         NetworkController.Instance.CreateNetworkPrefab(false, PlayerViewHandLeftPrefab.name, PlayerViewHandLeftPrefab.gameObject, "GameElements\\Player\\" + PlayerViewHandLeftPrefab.name, new Vector3(0, 0, 0), Quaternion.identity, 0);
42.                         NetworkController.Instance.CreateNetworkPrefab(false, PlayerViewHandRightPrefab.name, PlayerViewHandRightPrefab.gameObject, "GameElements\\Player\\" + PlayerViewHandRightPrefab.name, new Vector3(0, 0, 0), Quaternion.identity, 0);
43. #endif              
44.                     }
45.                 }
46. #if ENABLE_NETWORKING      
47.  

https://github.com/EstebanGameDevel...tion/App/Scripts/Controller/MainController.cs

Most functions have multiple compiler switches like that. It makes the code incredibly hard to read and maintain.

For future projects like this, I would absolutely advocate having separate classes/interfaces for drastically different modes like this across almost every method. I would personally consider this code to be very poor as written.

Sorry if this feedback is too harsh, but if you are submitting this code for a senior position, it's got a lot of issues.

 

I know that pre-processor constants isn't a nice solution but I don't see how you can support builds for different platforms whose libraries collide with each other. I'm curious how would you face a multiplatform (mobile(IOS,Android), desktop, VR(Oculus, OpenXR)) multiplayer(Mirror, Photon) project? Could you share some of your code code. Do you have any repositories where we could check your work?

I've seen the problem with the spaces, thanks. In the code editor it isn't visible but when I checked in the browser I saw the problem. I will adjust the auto-formatting of VS Code so that doesn't happen in the future.
Usually people check the code after clonning the repo with decent code editor (VS Code, Visual Studio). I assume either you didn't clone the repo or you opened with the notepad, not Notepad++, it works there, but not with plain notepad. I think you just checked it in the browser, I don't think you bothered even to download it. In my world checking the code of a project without clonning the repo an opening it with a real code editor is a deal breaker issue. It's disrespectful to criticise the work of the other without using the right tools. We agree in one point, we wouldn't hire each other.

It also would be nice that since you have mentioned you wouldn't hire me to know where you are working. Since you are so eager to teach the others I would like to know your work better. In your profile there is zero information about that. Also it would be awesome if you share your company's coding test. It would be nice to know that you are working in a decent company.

I wasn't really expecting feedback. I just wanted to share the code test so others could be prepared. You can use or not the code. I don't care. I understand that sharing the whole thing gave the green light for some of you to criticise, but, at least, if you are going to make a code review, bother to clone the project, use the right tools and share your coding experience to know what is your background. It's easy to criticise anonymously.

 

Hey, sorry if I offended you. My goal was not to nitpick or be an ass or start a pissing match. I thought some feedback would be useful, but it was either written too harshly or generally unwelcome.

 

Would be curious to see alternatives too. While I've not bothered with this much myself, code like yours is what I have seen in multi-platform supporting assets from the store.
Makes it less readable, that is true, but can one avoid it without introducing code duplication instead?

 

Use an interface for your platform specific class.
Have an initialization object in your init scene or when instantiating the network stack in this case.
In that initialization object check once for the defines and instantiates a game object/class based on the defines and store the reference to your platform specific object in a global manager and work with this reference.

Using platform specific defines all over your project makes for very hard to maintain code which would raise big red flags if I was reviewing an application for a senior role.

 

Using preprocessors isn't the problem. It's that you have them everywhere. Ideally you want to consolidate them into one location and have the implementations they select separated to make them more easily maintainable.

Here's an overly simplistic example of the pattern used by my last work project.

Spoiler

Code (csharp):

1. public class ExampleScript : MonoBehaviour
2. {
3.     private IPlatformSpecificAPI platformAPI;
4.  
5.     private void Start()
6.     {
7.         platformAPI = PlatformAPIFactory.CreatePlatformAPI();
8.  
9.         platformAPI.MyProperty = 5;
10.         platformAPI.DoSomething();
11.     }
12.  
13.     private void Update()
14.     {
15.         platformAPI.DoAnotherThing();
16.     }
17. }
18.  
19. public interface PlatformSpecificAPI
20. {
21.     int MyProperty { get; set; }
22.  
23.     void DoSomething();
24.     void DoAnotherThing();
25. }
26.  
27. public static class PlatformAPIFactory
28. {
29.     public static IPlatformSpecificAPI CreatePlatformAPI()
30.     {
31.         #if UNITY_STANDALONE || UNITY_EDITOR
32.             return new StandaloneAPI();
33.         #elif UNITY_IOS || UNITY_ANDROID
34.             return new MobileAPI();
35.         #else
36.             throw new NotSupportedException("Platform not supported.");
37.         #endif
38.     }
39. }
40.  
41. public class StandaloneAPI : IPlatformSpecificAPI
42. {
43.     public int MyProperty { get; set; }
44.  
45.     public void DoSomething()
46.     {
47.         // Implementation specific to standalone platform
48.     }
49.  
50.     public void DoAnotherThing()
51.     {
52.         // Implementation specific to standalone platform
53.     }
54. }
55.  
56. public class MobileAPI : IPlatformSpecificAPI
57. {
58.     public int MyProperty { get; set; }
59.  
60.     public void DoSomething()
61.     {
62.         // Implementation specific to mobile platform
63.     }
64.  
65.     public void DoAnotherThing()
66.     {
67.         // Implementation specific to mobile platform
68.     }
69. }

 

Really nice clean example of how to approach this.

This kind of thing can be tricky to do well on non simplified examples, but conceptually, this is how you approach it.

 

He doesn't want critique!
But I doubt minor code issues are the problem here (FYI I've got good jobs with poorly written code, because the demos or experience has the sparkle, not the code - YMMV!!)

 

It's very dependent on the company and the people conducting the interview as to how minor they are. Companies that have adopted enterprise practices will have very different opinions to ones that are focused on getting games completed and out of the door as quickly as possible.

 

doesn't a large company have standards for how to dress code up and what sort of classes go where?

if that was true, then wouldn't the only thing to test be general problem solving, interpersonal skills, or knowledge about some specific domain?

 

The company gave a coding challenge that specifically revolved around multi-platform stuff. It's pretty reasonable to assume they're looking at how you would deal with this specific problem in code. It would be part of "problem solving" - like can you solve this problem without making a giant mess.

Generally speaking there are some basic approaches that anyone who codes at a professional level should be familiar with, and that would generally include stuff like using interfaces for this kind of problem like @Ryiah provided above.

This should probably be around entry level skill for professional work, as this kind of thing should generally be covered in classes. But it's possible that standards are all over the place these days and much looser for dev jobs.

 

Agreed. I just checked the C# Yellow Book which is an entry level college textbook and interfaces are taught at around the halfway mark (starting on page 109) with factories (like the one in my example) starting at page 152.

https://www.robmiles.com/c-yellow-book

 

I've been thinking about this in the recent months as well. It seems the game dev community and the unity community in particular seems to be quite bad at defining common professional standards.

For example in the snipped above from OP I see use of the static singleton manager pattern. Can't fault OP for that as it is heavily used in the unity community.

In my view this is a lazy practice that promotes unnecessary coupling and has lead in our current project to terrible memory management and extremely hard to find bugs in production since just a single forgotten reference to a manager object can keep whole sceness or UI instances with referenced assets alive.
I've refactored big parts of the code base to purge it of static manager instances which.

It seems Unity is promoting this cluttering of practices with the engine API being all over the place as well with the mono-components, ECS, OOP (for example UGUI) to name a few paradigms in use.

 

OP sample is blamed for messy code, not for using specific pattern.
Also depends who are learning from, as Unity community is massive and holds in large portion less experienced devs, while proportionally fewer much experienced devs. So learning from general community practices, may not necessary be good a indicator of professional practices.

 

Ugh, I'm sorry to hear you had that experience. Yuck.

And, I would hope, on the circumstances of the "test". My code will be very different if it's the start of a long-term project as opposed to a quick prototype / demo which only has to last a day or two. I certainly wouldn't look at someone's "messy" code written in tight time constraints and assume that it reflected their experience level. It should be the starting point for a conversation, not the decider in and of itself.

Aside from the indentation, the state of the code Frosted pointed out wouldn't bother me in and of itself. I'd ask about it, and the direction of the conversation would be more important than the code itself. I've often had prototype code which became spaghetti. Yeah, it's hard to maintain and read, but no, I didn't care, because I had no plans to maintain it or to keep it for longer than I'd remember it anyway. Once I'd learned whatever I needed to learn, I'd use that to deliberately design a new solution.

Having said all of that, I wouldn't expect it of the average recruiter. If I were applying for a job I'd be giving them what I thought they wanted to see. Y'know, "when in Rome, do as the Romans do". Whether it's right and fair or not, you don't get an opportunity to debate how they opened it or whatever if they're essentially just using it as an arbitrary filter.

 

The indent snippet looks like some minor tooling issue. At least for me the big topic is the choice of pattern usage.
Developers become "senior" devs by following random patterns that get thrown around in the community such as excessive define use or static singletons that lead to issues in bigger productions.

I've hired devs including seniors in the past and we had to pay significant money rewriting code that was following these patterns to get the quality we want.
My assumption when reviewing code sent in from an applicant is that this is the best it gets.
Yes, perhaps I'd give the candidate a chance to defend their code in a subsequent interview. But then I'd need to hear a explanation as to why this code is a bad idea for production, which essentially negates the whole point of sending in this style of code in the first place.

It is a buyer's market for developer positions and as a company you also take a considerable financial risk when committing to hiring somebody so unfortunately it pays for companys to let applicants jump through a couple of hoops.

 

So I want to just add some notes about my original critique.

The reason tabs/spaces being included is problematic for interview code, isn't that this is BAD CODE and OP is a terrible coder. It's that it communicates a lack of care and attention to detail.

There are other things like that (without critiquing any of the code itself) - like the obj/debug and log folder being included in source control. This kind of thing is a red flag for someone who is receiving a lot of candidate code samples.

(It's probably a good idea to use a standard unity gitignore: https://github.com/github/gitignore/blob/main/Unity.gitignore)

Generally speaking, major red flags for sloppiness are exactly the kind of things that get people filtered out of tech interviews the fastest and are generally the easiest stuff to correct.

 

Do they? Personally, if they're only "following" then they're not really "senior", particularly if it's "random patterns".

In my opinion, someone is "senior" if they can develop their own solutions to new problems and, whether they're using their own solution or others', if they're able to select one solution from many based on how the pros and cons of each fit the specific use case(s). The latter may require experimentation, which is in itself a very valid reason for a true "senior" developer to produce "messy" code. And if they can explain the experiment the code came from and how it fits into a broader decision making process then that is far more important to me than how the code happens to look.

From a recruitment perspective, consider this: for any competent developer a defined set of code conventions is a trivial thing to pick up, a few days or a week. I agree that it's important, but it's not particularly rare, and it's cheap and easy to address. On the other hand, problem solving skills combined with experience to accelerate finding solutions are relatively rare, very rare if they need to be domain-specific, and are expensive and difficult to address because they take years to develop.

Though I'll reiterate, I wouldn't expect the average recruiter to see things that way.

 

Reading through his second post again about how it's disrespectful to criticise someone without using the proper tooling to check their work I decided to clone his repository and open it in Unity (after first installing his release).



It's a simple error. He's using a preprocessor called

ENABLE_NETWORKING

to enable and disable the appropriate boilerplate and logic but he accidentally forgot one and it was looking for

NetworkController

while the using statement was still being excluded.

I will make the assumption that he fixed this on his end but I'm not going to make assumptions about how this ended up in the repository just that it's possible the interviewer received the project in this state.

 

I've never seen anyone make it out of a junior dev position while still doing this.

 

I didn't look into the repo I just saw the snippet posted above.
The cost of ensuring auto formatting is used and teaching using a CI pipeline to detect code compilation issues takes days.
The cost of ensuring usage of what at least I consider professional standards takes months if not years. The later being the cost I have in front of my eyes when I see the defines.

 

I understand and accept that consider job trial code to be "as good as it gets", so I understand that perspective. However, putting that aside, do you consider there to be any difference in standards applicable to experimental code vs. production code?

 

Sure. Howevewer introducing mess as OP case, even as a prototype, it is not acceptable as a senior. There is lack of basics principles in given OP case. Ot shows.

It will reflect the rest of the code, when under the pressure and the 0time constraints.

As the result, it will affect negatively rest of the team, if working in such environment.

 

Sure, that code has no business being shown to anyone anywhere though.

A qick&dirty first implementation should still not get in the way of the inevitable second-pass rewrite.
For that to work the underlying architecture of the code has to respect proper pattern use.

Senior code provides the rails for the rest of the team, which would be general guidelines for whatever you send in when applying for a senior role.

On the topic of 3 days long tests, I'd personally push back citing scheduling issues and suggest focussing on a subset of the requirements based on the profile of the company and the role.
But that, of course, comes with the risk of alianating the recruiter.
Personally, I think it is a sign of a mature developer if you are able negotiate cutting the test down to a meaningful piece.

 

I would have asked them to test me within a 2 hour time slot, if they cant assess your skills in that time then they are likely not worth their salt.

A lot of companies that do seemingly abitrarily long tests like this are actually using you as a cheap source of labor, and often giving you prototyping tasks so they dont have to give it to developers.

Case and point I applied to a web role that had some WebGL requirements, and they asked me to create an entire CMS for them which had branding and info from a real customer. I told them thats not a test and withdrew my application. Found out later that they were just handing off low level work to applicants and then using the best result. For them it was a way of getting free contractors.

 

Everyone always repeats "singleton bad" and then doesn't follow up. Ask what the alternative is and all you get is it depends of the project and similar non-answers. So what is it? Service locator pattern, dependency injection, scriptable object architecture or something else? Each and every project can't have a unique static singleton alternative that's never done before and is never repeated again yet is superior to said static singleton.

I agree but you're also not making anything clearer besides singleton bad and Unity community is used to bad practices. What are the good practices then? Can someone finally clarify?

 

I don't have a problem with singletons but the OP's implementation has a flaw: you need to remember to add it to a scene you want to test or start testing from a scene that already has it. I wrote the following up in notepad so there may be problems with it.

Spoiler: Self-instantiating singleton

Code (csharp):

1. public class MySingleton : MonoBehaviour
2. {
3.     private static MySingleton instance;
4.  
5.     public static MySingleton Instance
6.     {
7.         get
8.         {
9.             if (instance == null)
10.             {
11.                 instance = FindObjectOfType<MySingleton>();
12.  
13.                 if (instance == null)
14.                 {
15.                     var go = new GameObject();
16.                     go.name = "MySingleton";
17.                     DontDestroyOnLoad(go);
18.  
19.                     instance = go.AddComponent<MySingleton>();
20.                 }
21.             }
22.  
23.             return instance;
24.         }
25.     }
26. }

For fun I tried asking ChatGPT what an alternative would be and when it suggested dependency injection I told it to write me a system to do it. I would still have preferred a third party solution like Init(args) but the OP's requirements forbid third party frameworks aside from platform dependencies so this would work with some minor modifications.

Spoiler: ChatGPT DI

Code (csharp):

1. using UnityEngine;
2. using System;
3.  
4. public class DependencyInjector : MonoBehaviour
5. {
6.     private static DependencyInjector instance;
7.  
8.     public static DependencyInjector Instance
9.     {
10.         get
11.         {
12.             if (instance == null)
13.             {
14.                 instance = FindObjectOfType<DependencyInjector>();
15.  
16.                 if (instance == null)
17.                 {
18.                     GameObject container = new GameObject("DependencyInjector");
19.                     instance = container.AddComponent<DependencyInjector>();
20.                 }
21.             }
22.  
23.             return instance;
24.         }
25.     }
26.  
27.     private readonly Dictionary<Type, object> dependencies = new Dictionary<Type, object>();
28.  
29.     public void Register<T>(T dependency)
30.     {
31.         Type type = typeof(T);
32.  
33.         if (dependencies.ContainsKey(type))
34.         {
35.             Debug.LogWarning($"Dependency of type {type.Name} is already registered.");
36.             return;
37.         }
38.  
39.         dependencies.Add(type, dependency);
40.     }
41.  
42.     public T Resolve<T>()
43.     {
44.         Type type = typeof(T);
45.  
46.         if (!dependencies.ContainsKey(type))
47.         {
48.             Debug.LogError($"Dependency of type {type.Name} is not registered.");
49.             return default;
50.         }
51.  
52.         return (T)dependencies[type];
53.     }
54. }

Code (csharp):

1. public class GameManager : MonoBehaviour
2. {
3.     private void Awake()
4.     {
5.         DependencyInjector.Instance.Register(new ScoreManager());
6.         DependencyInjector.Instance.Register(new AudioManager());
7.     }
8. }

Code (csharp):

1. public class PlayerController : MonoBehaviour
2. {
3.     private ScoreManager scoreManager;
4.     private AudioManager audioManager;
5.  
6.     private void Awake()
7.     {
8.         scoreManager = DependencyInjector.Instance.Resolve<ScoreManager>();
9.         audioManager = DependencyInjector.Instance.Resolve<AudioManager>();
10.     }
11.  
12.     // Use the resolved dependencies...
13. }

 

Ah, finally someone delivers something actually informative. The ChatGPT DI solution is interesting and remarkably simple to understand compared to DI frameworks I've looked at.

 

Easy for me to say as I'm not looking for work, but I'd not care about alienating them with a perfectly reasonable request. Under ideal circumstances teh interview process works both ways - you're also deciding if you want to work for them. And if they're unreasonable then my answer is no.

 

You can also make the base singleton class generic so that you have one base class handling the singleton functionally, and derived classes represent new types. Passing the derived type in as the generic type parameter to the base class accomplishes this.

Also, if you like that DI solution, you should check out my DI plugin (when Unity approves it). I admit that AI generated DI service locator is slick and simple. However, mine does the same type of compile time type inference, no reflection, but it lets you define the backing implementation, rather than just defaulting to a dictionary as a dumb terminal for backing storage. Also, my method for resolving dependencies also has generic type inference and returns a boolean, so it can be included with other conditions on a single line with a corresponding execution block. So, I think it looks nicer from where it gets called.

 

Ryiah's DI version is one way of approaching it.
It is better than MySingleton.instance but still has memory leaking issues since removing the reference is not cleaned.
Also if you want to maximaze performance DI is typically not the best.

My default pattern is to have a pure static class storing references and a constructor for it.
I typically don't use MonoBehavior for it and split the construction from the destruction but for the sake of simplicity I've put it together here.

Code (CSharp):

1. public static class MyModuleEnvironment
2. {
3.     public static IMySingleton MySingleton;
4.  
5.     public static bool EnvironmentInitialized = false;
6. }
7.  
8.  
9. public class MyModuleEnvironmentConstructor : MonoBehaviour, IModuleEnvironmentConstructor
10. {
11.     public void OnEnable()
12.     {
13.         GameObject mySingletonGameObject = new GameObject("MySingleton");
14.         MyModuleEnvironment.MySingleton = mySingletonGameObject.AddComponent<MySingleton>();
15.         MyModuleEnvironment.EnvironmentInitialized = true;
16.     }
17.  
18.     public void OnDisable()
19.     {
20.         MyModuleEnvironment.MySingleton.Cleanup();
21.         MyModuleEnvironment.MySingleton = null;
22.         MyModuleEnvironment.EnvironmentInitialized = false;
23.     }
24. }
25.  
26. public class MySingleton : MonoBehaviour, IMySingleton
27. {
28.     public void Cleanup()
29.     {
30.         Destroy(gameObject);
31.     }
32. }
33.  
34. public interface IModuleEnvironmentConstructor
35. {
36. }
37.  
38. public interface IMySingleton
39. {
40.     void Cleanup();
41. }

Two big things here.

Since MySingleton is in option now actually cleaned (static set to null) the GC is now allowed to clean all memory linked by MySingleton.
In case of MySingleton inheriting from MonoBehaviour this includes the full transform child/parent hierarchy of the linked GameObject but could also be loaded assets for example.

Also now since you are actually defining discrete modules containing references to your manager classes code dependencies are much more visible.

In case of the MySingleton.instance it can be extremely hard to have an overview of what major classes your project actually contains since there is no files that actually list them.
This is not a problem when working on a small project (aka tutorials or samples that get fed to beginners to learn from), but when you have dozens if not hundreds of MyStingleton.instances in a serious production this instantly becomes a giant mess.

Generally even for singletons I use interface. This makes working with the classes as a consumer easier and this allows you to provide different implementations for example for a platform specific network manager.

That being said it is still true that your project may have requirements that make this option not optimal. There is no one size fits all solution here.

 

Generally speaking I think everyone will have their own flavor of this kind of thing.

If I were hiring senior devs, I would not dock someone for any approach that at least looked like it was something they had at some point put some consideration into, and picked a flavor of solution that they thought worked well.

Something centralized or particularly well organized, I would probably give extra points to, likewise something that looked haphazard I would subtract points from.

Generally speaking, this is also kinda how I would judge the code itself. Code that reads as a nice clean centralized reference is generally better than code that's splintered and scattered. Same principal.

____________________

One of the main things when interviewing seniors, is that you're looking for evidence that they've got the experience of taking a naïve approach to a problem - learned the pitfalls and developed a better pattern/habit for future dev.

This is also why something like including obj/debug in git is a red flag. Because, you'd expect a senior to have experience setting up git, and if you have everyone pushing their obj/debug folder and conflicting, that's obviously a problem. It's also a problem that you will tend to remember next time.

So seeing stuff like this is an indication that the applicant hasn't actually set up source control before, which means they're probably fibbing about their experience.

Same generally for a singleton approach, DI or singleton or whatever, there are problems and issues you run into w a naive approach. You expect a senior to have settled into patterns/habits that prevent those issues (you can forget to drop an instance in scene, so make it self initialize. You can forget to clear the reference, so have some method for assuring that's clean, having tons of stuff can become a giant mess in large project so some way to centralize stuff, etc).

Generally speaking, what you're really looking for reviewing code samples like this - is 'how many of the annoying issues does his/her code address?' and from that - you are judging how much experience they really have, vs how much fibbing they're doing on their resume.

 

If I'm not mistaken you can make git ignore differences in whitespace but if it's not set to ignore it any lines that are screwed up will be added to the diff which means going through past changes will be more difficult when you have lines that aren't actually changed but simply had whitespace adjustments.

While I don't make a habit myself of going through diffs before making a commit I do like to verify from time to time that the repository doesn't have weird things happening in it. Just relying on the editor to do the job for you can be problematic.

How careful they're being with their projects too. I'm not going to say that mistakes don't happen, and I've made my fair share of commits with things that shouldn't have been committed, but you don't want to rely solely on the tools to do the right thing. It's one of the many reasons we have version control after all.

Edit: I've removed a section of my post after reading responses by others below, so if anyone wonders what they're commenting on it's that.

 

Discussing it in general terms is one thing, but it feels a little rough to see someone's work specifically criticised after they've said they don't want feedback.

 

Even rough, it is perhaps an opportunity to improve skills and see the issues, which has been unoticed, or ignored, maybe even unknowingly so far.

People gets into habits and tends to stick to them, until someone points out, there are alternative routes. And perhaps even better one.

 

Yes, I can understand that and I wasn't positive if I wanted to make that post, but on the other hand he stated that he uploaded the code for the purposes of benefiting others and it's not very beneficial without any context telling you whether what you might learn from it is good practice or not.

Like I mentioned before his code is fine if you just want to ship a project, and he clearly was able to meet the requirements in the time frame. In fact the document the company sent is in that repo (with references to the company removed of course) and it suggested a longer time frame. Up to one week if I recall.

 

Right, but the good stuff you both mention can be achieved without calling out specific people or their work, which is the bit I took issue with.

 

Wholeheartedly agree! Although as forum regulars I am sure we both know that its pretty rare to see anything that could be seen as antagonistic posted by @Ryiah who is often one of the more level-headed regulars in terms of posting etiquette, so I am sure its an honest mistake/misjudgement

 

I definitely agree there.

 

It might be rough for the OP but I also find this to be one of the more useful threads on here in recent times. It's rare to have intermediate/advanced level of code be critiqued on this level with proper solutions presented also in code form rather than just theory. But theory has also been great.

 

Would be even better if OP returned and explained reasons why they did things the way they did. Might be something critiquers weren't aware of. Everybody might learn something - usually a lot to learn from understanding the problems encountered on some endeavor. Critiques are easier to make than it is to create, especially when working on a tight deadline and for no pay.

 

I decided to just remove that section of the post (and left an edit behind with a brief explanation of why so that the posts that follow don't seem out of place). I don't think anything else is glaring about it but if it is let me know and I can adjust it more.

 

You rock, @Ryiah!

I also removed it from my quote.

 

As much as its nice to be kind to OP. IMO, holding back detailed critique is a disservice to both OP and others who might be following along. Even the act of critiquing itself can be a learning experience and self reflective.

For example, after s**ting on op's sloppy git submissions, I finally got around to removing a few files from my own repo which shouldn't have been included and updated my gitignore.

I've written tons of s**tty code over the years, you all have too. We're all human, and there are times when everyone will cut some corners or slop their way through something really annoying. And even if you know better, sometimes you need a reminder that letting too many little things slip past can start to wholistically degrade the quality of the work.

If you care about your craft at all, being open to critique, even if you choose to disregard all of it, is almost always a good practice to engage in.

Ultimately, you get out what you put in. I s**t on OPs git stuff, then reflected a bit on my own work and made improvements.

If OP is still submitting code to jobs, I hope he also uses some of the critiques in this thread to tighten his own stuff up. Ultimately, this will help him get a job faster, or land a better job. As much as job hunting sucks, its really the worst time to be closed off to unpleasant feedback.

 

Who would ever even consider working for a company so disrespectful of your time they require a three day test, on top of all the other H.R. hoops you have to hop through just because their prospective employee evaluation team cant come up with a half-day AT BEST test?

What a bunch of amateur hacks.

Its a clear sign they want to chew you up and spit you out.

 

Speaking of feedback if you believe you've been ghosted try contacting the company again after sufficient time has passed that you think they've had an opportunity to evaluate everything you've sent them. If more than one person was involved in the interview you could even try reaching out to one of them.

It's possible it's as simple as the person handling communication outside of the company has had an overwhelming number of candidates and may have just missed you. I've had my own problems where emails have become buried for one reason or another and a subsequent email got through to me.

Also ask for feedback on the interview itself. You might just get ignored or you might get some valuable information that can assist with your next interview.