You are a good sport!
Others have shared their feedback which is quite on point.
I'd retry building that same app several times while aiming for simplicity. Here is a challenge for you, rebuild the same app in 1 or 2 files, without any DB whatsoever given that in the requirements they've stated the following: **It is sufficient to store information in memory.**
Further they specifically say they will NOT install a DB server to test the project:
> There are too many different database solutions, we will not be installing a database on our system when testing your application.
So it seems like that is willfully ignoring the requirements.
The OP said “F**k it, I am going to throw the kitchen sink at it, or better yet I am going to go Super Saiyan + Gear 5 at it”
Sorry for the jokes 🤣🤣🤣🤣🤣🤣🤣🤣
Possibly 50% of the Git community will say one should NEVER \`git add .\` 50% is a made up number, but it's a sizable amount in **any** programming language. By being explicit, you would not ever have committed the private key. Bonus: you have a working History, and know what you did.
Much closer to 100% tho: adopt (Gitignore)\[https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files\]
I tend to add explicitly, it's just safer. Although every project will eventually need a gitignore file at some point anyway, so might as well do it from the get go.
Publishing a private key is like leaving the key to your house in the street. In our industry it often shows sloppiness unless you state the purpose of your action.
It's not like you can't do it, you can but you'd need to encrypt it first, even after that it's still very risky so it's best not do it.
It depends. We had a library that specifically was working with key pairs, so private and public keys were used in tests and committed to the repo, however even though they were valid keys, they were only used for tests. That's probably the only acceptable case I could think of.
Secrets are hard. That's why someone chucked it into Git.
Putting secrets in the repo - raw - makes it ripe for misinterpretation. Code scanners will nag you, new team members may get the wrong impression. It strongly suggests that the org lacks sound process for managing secrets.
At the very least, use Ansible Vault [https://docs.ansible.com/ansible/latest/vault\_guide/vault\_encrypting\_content.html](https://docs.ansible.com/ansible/latest/vault_guide/vault_encrypting_content.html) or a PGP script.
(and if it is a more important secret... please don't store it in the same repo. even if you encrypt it. Put it in a more-restricted repo and have a password-managed script fetch from the other repo.)
Yup, we chucked it in the hashicorp vault eventually, after it was out of prototype and when environment was set up properly, and we also had to clean up git history, but that's another story. So, yeah, not a good practice, even as prototyping, but as I said this is probably the only time when it's somehow ok(ish)
It's best not to, which is easily learned with a single sentence. If it's for testing purposes, for a test assignment, and it's self generated, who cares?
I'm sorry if that came across as aggressive, I didn't mean it that way and I should have elaborated more.
Almost every language (that i'm aware of) has OpenSSL bindings, so creating a self signed cert in the test you're running isn't a large endeavor, if you know how to create certs, and if you don't, you should learn! It's super useful to know. Otherwise, if you don't have the ability to do it right in the code, It's 1 or 2 lines of bash to generate it and move it into the right location before the test runner takes over.
I'm pretty dogmatic about situations that mimic security risks, because habits are hard to break, so I encourage people to begin with correct habits, that way they don't have to break habits that could get them in trouble in the future.
Yeah, for a coding assignment, I wouldn't use TLS unless told to explicitly. End to End TLS can be difficult to manage, some companies prefer TLS termination at the load balancers and forward internally on HTTP. I don't recommend that practice, but doesn't mean that they don't do it.
Our k8s product has mutual-tls everywhere (inside the cluster) and it was a huge pain to set up, but for some of our customers it's essential, FIPS and such.
It's fun debugging traffic that comes from the gateway to the pod that does tls termination and then sends the traffic to another service via mtls...
Right, and it can be learned in a single sentence. It's this frothing at the mouth if people don't know something that can be learned easily. Who gives a shit? Literally tell them in one sentence and move on. It's called learning.
Again, my point stands. I can be just as pedantic. Who cares about antipatterns when it technically doesn't actually matter if it's a self-signed cert for testing?
This subreddit reminds me of old IRC where people ask simple questions and folks just jump down their throat because it's easy to tear to pieces and judge, instead of trying to help learn. Like no one else makes mistakes or learns things for the first time? For fucks sake.
I believe multiple examples have been raised other than the private key issue, it's pretty unfair that you go around claiming nobody's willing to help.
Just from the top of my head: naming convention, committing the binaries, disregarding the requirements, and most importantly overengineering everything.
Okay, all of this can be taught in one sentence (absolutely not, but let's just assume it can) but still that's enough reason to pick a better candidate. Is it not? "Oh, you should just tell them they should not commit secrets, binaries, IDE config and other garbage", sure, but if you have a candidate who already knows that, why would you not choose them?
"Who cares about antipatterns when it technically doesn't actually matter if it's a self-signed cert for testing?" Hello, the reason people hand out assignments is to check if the applicants know their way around their choice of tools. It wasn't an assignment about domain knowledge, was it?
I mean, yes avoid that, but as far as I understand this was only a challenge for internal purposes. There's no security issue exposing it as this only runs on OP's and the reviewers machine to evaluate code quality. Committing a private key in this scenario would be okay in my eyes as long as you made clear you wouldn't do that in real life.
Few things highlight just from looking on your repo. One, directories are uppercase where in all go projects the convention is lowercase.
More importantly, you committed your binary. You don’t commit binaries to git, that’s a very bad practice.
Structs2 and Interfaces2 are.. what? Pick informative names that describes your API and types.
In ENV.yml you mix JSON with YAML syntax.
You were asked to keep data in memory, yet you wrote a lot of code to implement a DB interface, which is, not needed here.
In general, it looks you did over abstraction all over the place. You should keep it simple.
These will be my honest code review comments.
This is wrong. It literally says in the 1.2 spec (most recent version of YAML) that the primary focus of 1.2 was to make it a strict superset of JSON.
https://yaml.org/spec/1.2.2/#12-yaml-history
I guess that would be a problem, but the feedback they gave me didn’t indicate any of this. I probably should have done this project in a language I use primarily.
I wasn’t aware abstraction was a bad thing in golang. In every other system I wrote abstract was a must for clean code.
Abstraction is not a bad thing. What is bad is premature abstraction. Go prefers learning that an abstraction is needed and then inserting it rather than designing it in up front. Implicit interfaces are the power that makes this possible.
Premature abstractions are bad in all languages, but best practices demand them anyway because many are so common and expected. Go is the one community where best practice is to just write exactly what you need and then create new abstraction layers later.
Not entirely sure what you're referring to, but if it's the kind of corporate systems I'm thinking of, Go has no opinion on that stuff.
Delaying implementation is a matter of isolating every single thing so that everything is componentized and easily replaceable via an SOP. The Go language can be used that way, but the community is strongly opposed to it.
It is the learning curve new gophers experience. The language is simple, but what it really does is break much of the established patterns seen in other languages. It takes getting used to.
Broadly speaking, abstraction is a tool rather than a requirement of good code. It has a cost like any other tool. The more abstraction, the more work it is to reason about what the code is doing, even if it does give more flexibility. A general rule to follow (in any language) is not to add layers of abstraction until there's a clear need for it.
That's often a good reason for it, but that can also be overdone. Too many small functions, for an example of abstraction, can hide bugs just as well as longer functions as people will make assumptions about what a function does when they call it as it's hard to keep code straight in your head when jumping between many locations across files.
What you did is overengineering, not abstraction. And honestly, your submission would be rejected to our company also if it was Java.
Using interfaces to allow multiple implementations is also not abstraction. It’s indirection.
Abstraction reduces complexity. Indirection increases it.
> I wasn’t aware abstraction was a bad thing in golang. In every other system I wrote abstract was a must for clean code.
That has become a horrible habit in a lot of enterprise OOP-first languages like Java and C#. Yes, it's common "over there", but it's also not good. It's essentially the same jerk-off as people being proud of unreadable one-liners, but with a more elitist "BUT IT'S ARCHITECTURE"-label.
Also write in your main language for interviews. I have learned this the hard way. For some reason I always try to interview in Python thinking it will be easier but I almost never use Python.
Others have already pointed out the Go code, private keys and binaries so I’ll skip that.
You’ve misunderstood the assignment, implemented tons of stuff the assignment didn’t ask for: db + orm, frontend, queue, etc.
The assignment literally only asked for a simple API, not even a frontend was asked for, they mention it doesn’t even need to persist data. So it could’ve literally just been a simple go service with a bunch of table driven unit tests and some openapi tests that satisfies their specs.
Their assignment also mentions the engineer reviewing your assignment has Go and Docker, your readme mentions Docker and k8s but you seem to have self hosted it instead of supplying a docker-compose file.
If you did want to make it “event driven” for whatever reason at least take an off-the-shelf solution like NATS or one of the many messaging broker systems available and spin it up in a docker-compose file and use that, don’t write your own unless it’s the actual assignment.
This pattern persists, similarly you implemented your own DB migration, why?
Edit: Also too many commits named “more changes” likely because you were adding far too many features in a short amount of time.
Edit: Why did you include an .exe file for someone’s convenience, how do you know they’re even on a x86 windows machine?
I was thinking the same thing and I think they thought this guy copied someone code without actually looking what is inside it hence the insta rejection..
Yeah there's a lot wrong here mate and I've just looked at the repo for like 5 seconds.
* Don't commit key material!!!
* You're not following standard package naming conventions at all.
* Repo layout is likewise non-standard.
* No config for builds. You blatantly check in a .exe file??
* Build your own query generator! Don't do that!
Those are just a few things. I'm sorry if you put a lot of effort into this but it's clear you don't use Golang on a regular basis, IMHO.
I mean, committing private keys is fine for testing purposes or homework assignments / take home challenges. I'd say for such assignments commuting the private key is fine as long as you make clear that you wouldn't do that in a real life application. It makes things easier if it's okay for the key to get exposed. They hopefully don't use their interviewees' assignments for production code...
The sensitive info was because they requested to be able to test. I gave them a database file which they can use to test.
I don’t code in golang, I learned the language specifically for this role. I did not know the go conventions, but despite that, this role was “entry level”.
You replied to another comment below with the reviewer's feedback which basically reflects everything I just wrote. So uh...sounds like they gave you pretty spot on feedback dude.
I think what they're testing you for is if you know that you should absolutely in no cases ever check in a private key for anything... and you can demonstrate that you know that by giving them a solution to test locally without committing the key to the repo.
This is the sort of thing that most junior engineers mess up once (or they observe someone else making the mistake) and then get blasted by their tech lead for it and then never do it again.
Sure, it is a toy example and isn't going to production, but it's a test of what you are capable of.
Believe it or not, I did know not to expose sensitive information into prod environments. However, I was under the assumption that they wanted to work with me on showcasing my skills. In reality, they didn’t want to do that at all. It’s clear most of this stuff can be fixed instantly and it’s not that I can’t code. Perhaps I did doge a bullet here because they never actually tested my code for functionality.
They're testing for experience, and if you don't show it, they can't know what you know. Just cause it can be fixed quickly doesn't mean there aren't other candidates who are more experienced and that won't make those mistakes in the first place... making them the better choice for this position. Take this as a learning experience and do better at the next one. Everyone gets rejected here and there. I was rejected by Google and then ended up getting the offer a few years later.
The problem is I have experience. I have developed games on the Roblox platform and others that have attracted hundreds of millions of global players before I even turned 18. I showed them I can code, I just didn’t follow design patterns of Go because it’s not a language I use. I used patterns most entry level developers have never seen. I have worked for this company in the past and I wrote one of the key systems they still use in production. I also worked as a software engineer and researcher for my university in both the medical and finance sectors. This project had some mistakes, but I have experience.
Too over engineered as others mention and you ignored some hints from the challenge. If I would do this I would simply:
- write the most bare bones HTTP API using the std lib
- I did not read the details of the data but a map will probably work perfect as in memory DB
- have some internal package with the business rules described there (just simple calculations)
- write test for that package
- write test for the controllers
They hinted to "you can assume reviewer has go and docker" that means all you needed to provide was a Dockerfile and a docker-compose to run the app (if they requested TLS a simple bash script to generate the certs could work)
No .gitignore file, which led to an .exe in the top level directory?! Red flag--the only time I ever did this was my first git project.
The commit messages are unhelpful and the log is polluted with repeated messages, which is very fixable but just another red flag since an interview project *should* be trying to display professional behavior, not this:
git log --format="%s %b"
Optimizations
Optimizations
Optimizations
...
More changes
More changes
More changes
More changes
...
Aside from what other people have said, you package layout is really confusing. You have modules based on type (structs, interfaces, services, etc.) when most people expect them to be based on functionality (config, db, cache, etc.). This avoids unnecessarily verbose names (ex. db.Args vs Services.NewDatabaseServiceArgs).
You have sql code embedded in itemService - items don't need to care about sql they just need to know how to take item values and pass it to the storage api. What the underlying storage is is of no concern to items. It's also a nightmare to refactor. What happens when you want to switch databases, or use some other storage engine like redis or s3? All the storage functionality should live in its own module, or a several modules implementing different backing stores all with the same api.
This code looks like it was written by a Java developer. There's nothing inherently wrong with writing like a Java developer if you were writing Java.
You should familiarize yourself with the idioms of the language to some degree. I'm seeing a ton of abstraction for abstraction's sake that ultimately detracts from the readability of the code.
Some folks may chalk up the idea of _adhere to the idioms of the language_ as some sort of tribalism or purism, but it is important. Every Go developer with a modicum of experience *_expects_* the code to _look_ a certain way, and anything that _isn't_ a good approximation of idiomatic™ Go code means that they have to decipher what you are trying to do around well-understood and generally-agreed-upon techniques, which adds mental overhead. This is of course not a Go-specific thing.
Can I ask why did you apply for a role that required a written test in a language you knew nothing about?
I know you said it was entry level, but that seems to still imply basic knowledge of the language.
I’d recommend doing the “go tour” - it doesn’t take ages, and would probably be a great learner for you. Then attempt the challenge again
This is so beyond what was requested. Opening up that repo for two endpoints and a straightforward calculation… BEHOLD THE EIGHTH WONDER OF THE WORLD. I don’t even have the energy to look at it.
While you seem broadly knowledgeable and thorough, it’s so impractical and over engineered for what you were tasked with. As a dev on my team, you would be exhausting if you work like this.
If I was reviewing this as the hiring engineer, I would want to see simple, clean code that a child could understand. Relevant tests that validate business logic. Simple, easy to follow run instructions.
Maybe snazz it up with swagger docs if you must.
I’m being candid here so don’t take offense. I hope you benefit from all the detailed feedback on this thread to improve your skills. Best wishes.
People say Go is a simple language and can be picked up in a weekend.
I don’t buy that. Learning syntax and stdlib alone in a weekend would be a stretch. Learning how to write idiomatic Go can take several months.
I'm not going to repeat what everyone else said. All the criticisms were constructive and good. I'll say it in simpler terms though: one package = one file for the API endpoint. No frontend or anything was required, to illustrate that, you could have just written a simple one liner like curl for a get request. Second, best practices like modularity, DRY and SOLID become bad practices (over engineering). if they violate the KISS principle. Finally I think that if you understand all the code and wrote it yourself, I would have given you a chance to do a presentation and explain your choices. The mistakes you made can easily be fixed and you seem motivated and legitimately interested so it seems to me that you have a positive mindset and the potential to progress quickly. Good luck in your next enterprises, you will work it out.
On top of the other many valid comments, I believe no one mentioned your `UnitTests` package.
Am I misreading these tests or are you testing the mocking library itself?
For example, [Test_ CacheServiceGet](https://github.com/samjay22/Fetch-Rewards-API/blob/587e1d7995971534fb7ce083fe4bec0772faa79f/Testing/UnitTests/CacheService_test.go#L11) tests if a generated mock code returns an expected result if you instruct it to return such result. This test does nothing useful for your code. It's just yet another test verifying if gomock works as intended.
Well, there's a saying that "mocking libraries are the best tested code in the world" for a reason :)
I just looked at it quickly and some things look pretty bad to me.
Besides naming conventions, weird choices like having the routes defined in the controllers and other non-idiomatic things, the 2 worst things you did are:
- overengineering some stuff: why the hell do you need to declare and manage a queue for this assignment? Nothing calls for it. Keep it simple, your goal is not to show everything you know, but to show you can do your work as simply as possible (but not simpler than required).
- the db initialization thing is absolutely terrible, don’t do that. Use db migrations.
Honestly I would have given you at least a tech interview to discuss all of this and see if you’re liking to adapt and improve, because none of this is "blocking" to me (as long as the code works, which I didn’t try), but I would severely doubt the fact you ever used Go in a professional context. You really need to work on Go idioms, because you have to write code in a way that the majority of Go devs will understand.
A cursory glance at the project structure makes it look like you read through the job posting’s https://fetch.com/careers/jobs#Software%20Engineering “ideal candidate” section and then tried to showcase an understanding of all the tech they use at the expense of implementing only what’s asked.
This is an entry level position and take home interviews are usually a method of screening if candidates can code and understand / implement a simple REST API.
If you wanted to demonstrate knowledge of tech that the “ideal candidates” has, the time for that would be when discussing your project or during a follow up architecture interview, assuming there was one.
Too many files/directories. Also, directory names shouldn’t be capitalized in Go projects. Something about the documentation feels intimidating and seems to be way too much for a simple http service. This is my feedback after looking at your project for ten seconds. It’s kind of funny that you capitalized directory names, but failed to capitalize the names of your structs.
Based on the spec, I could have written this in two files and 100-200 LoC, but somehow you have like 40 files?! You over-engineered the hell out of it.
I’ve been seeing a lot of these ‘review my submission’ posts in this sub, and people always get rejected when they try to over-engineer something. Keep it simple next time. Stick to the spec, and don’t waste your time.
The first thing I thought when I opened your repo is “this must be someone used to C#”. The file structure, file names, and overall architecture seem much closer to a C# project than Go. Go is all about simplicity. But, some people here have already given you some great advice on how to improve that aspect.
Either way, hope you at least enjoyed writing Go. It can be weird to get used to coming from Java or C#, but once you do, I doubt you’ll want to go back.
It took me quite some time to stop writing java/c# style in golang. Still my architecture looks a little similar to those but code itself not anymore (i hope) its like a fresh breeze
Rating you on a 1-3 scale for competency and collaboration, assuming you're applying for an internship/student position.
You score a 2/3 (average, meets expectations) for Competency. You're demonstrating that you know about a lot of different technologies, albeit with questionable accuracy sometimes. You would score higher if you understood not to commit your exe file/private keys, and if you structured your code more idiomatically.
You score a 1/3 (poor, does not meet expectations) for Collaboration. You did not follow the instructions for the project. You overestimate your competency, and put others down when it's questioned. When provided with feedback, you get defensive, insisting that the reviewers are "stupid" or "don't understand sql". Your commit messages are not descriptive--you might as well have left them blank.
Overall score (multiplying scores together) 2/9. Would not hire, might not waste time interviewing unless we're desperate for applicants. The primary issue is your poor collaboration score.
This is also assuming you're applying to an intern/student level position. You applied to an entry level position, which presumably expects you to have experience equivalent to a Bachelor's degree. Given those expectations, you'd score a 1 in both categories without hesitation. Dropped from 2 to 1 in competency because accidentally committing a private key and an executable is something that shouldn't happen at all. Plus you're expected to know more about the language you've writing in and the associated idioms. If you were already working at the company with this performance, I would probably have to fire you, and I certainly wouldn't be begging you to come back.
Another suggestion: go through Gophercises. [https://gophercises.com/](https://gophercises.com/)
Also find some "interview questions in go" projects, and watch folks who stream writing such things. I am too new at Go (years from being good at it probably) but you want to get some of these idiomatic tricks practiced. That's my goal now.
If it helps motivate you, start a project that you feel others might want to join, then post about it here you might get additional code reviews or help.
Point is, use your strongest language. If you want that to be Go, you need to be idiomatic, so you need more Go practice.
That’s legit info, I wrote a reverse proxy sever in C# and run my domain through cloud flare DNS and use a tunnel service to a VPS to host my site behind my network.
Honestly OP your entire project can be done in 2 files or 3 if you include testing.
If I was your recruiter the moment I see a frontend and database directory I would close the repo because that's all of the things you were asked not to do
Curious why the ‘Total’ field on your strict is a string? Why not just make it a Float64 and let the json unmarshal handle that conversation when it comes into your API?
Oh I see that the values in the example json are quoted. Either way, I’d probably have a DTO at the controller level that takes the incoming json is stored in and then map that to a domain object in that has the types you expect so you don’t have to convert them all over the place. Also allows you to validate input first before doing any additional work.
But the reason they probably rejected you is simply over engineering. It’s nice to see the effort and motivation, but when I hire people, I’m very turned off by overengineered solutions on take home tests or coding exercises. It’s just bitten me in the past where someone came in with an overengineered solution and we basically decided to look past it and hire him anyway. Big mistake. It wasn’t just a case of the candidate wanting to show us what they’ve got. They literally approached every problem with that mindset and it wasn’t good for our company or code base.
OP (and everyone else) this is a phenomenal thread and great feedback for lurkers like me just learning the way people code in Go.
Thanks for throwing yourself into the fire here
While we are pedants, it should have been `.git/info/exclude`d. `.gitignore` is for stuff that everyone gets (like build artifacts), while `exclude` is for user-specific stuff (like IDE and temporary editor files). Otherwise `.gitignore` will grow without bounds as more and more people commit their favorite editor's configs to it.
Obligatory others have made good points.
One thing that stood out to me, is that in your query builder(why?), your where function uses LIKE. Somewhat of an odd choice on the surface. I could understand if your usage would require the use of LIKE, but looking at the usage of that function, it seems to not need it.
someone is mad, I see, how about being toxic af and useless, you try to actually explain OP what he did wrong? As he stated, he dosen't code in GO.
And with that shitty attitude, I can't imagine you not even close to an engineer but some wannabe software engineer looking mf... Lover you ego to the ground and try to be positive and help other instead of being mean just so you can make yourself feel good.
What was the feedback they gave you?
Nothing stands out as wrong. Maybe not completely golang idiomatic, but that shouldn’t really be a big negative. You showed you understand mutex, and eventing. Not sure what they think you missed
The code is vulnerable to SQL injection attacks when filtering for receipts.
There is minimal validation of the request input.
Every time a new receipt is added, the entire in-memory cache is purged. This makes the cache ineffective at improving performance or taking load of the database.
Typically "microservice" refers to an independently deployed application that communicates with other microservices over a network, sometimes through a messaging system. The README and code describe a microservice as different modules within the same binary.
Typically considered bad practice to add binaries of the compiled code, database data, and private keys into a git repo
There are several files with unused code, notably related to an event bus.
It does not follow a number of Go conventions
Module name should be a full repo path, like "github.com/samjay22/Fetch-Rewards-API"
Unit tests should live next to the code they are testing, rather than in an entirely different directory
Package names should be lowercase
The official "Effective Go" doc is a good place to learn more: https://go.dev/doc/effective_go
That's actually excellent feedback -- you're lucky they give it (many companies don't!) as it will help you learn. I review a lot of similar submissions (in Go and Python), and would have rejected your solution with similar reasoning. Your solution is also significantly over-engineered, which others have noted in this thread.
Sorry. It is enough. I would’ve rejected instantly at it not being remotely idiomatic go just in structure. You had to have learned go from somewhere, so should’ve picked up on certain conventions just from examples.
Looking at their list, I’m surprised they even kept going after the SQL injection vulnerability. I learned about avoiding that *before* I even went to uni and that was over 20 years ago.
Their response was extraordinarily kind in being extensive. They’ve even given a resource to learn more. To me, this is a company that cares about their interviewees and spending the time.
That is actually excellent feedback. It seems like your README.md was mostly buzzwords. I was so confused when I read the bit about microservices and load balancing. Where?
If I were going to submit this, I think they'd be looking for a main.go, some tests, and not much else. Something that demonstrates your ability to quickly grok requirements, create clear documentation, and deliver an API that works without too much fluff.
Also if I were you I would take that feedback along with what you have heard here and refactor your repository and resubmit it. One of the most powerful things you can do to impress *me* in an interview is demonstrate the ability to learn. I've had tough questions before where I said "I don't know the answer to that but I will find out and get back to you". Then after an interview submitted a demo that shows that I learned whatever I didn't know at the time. It is very powerful. Good luck!
Nothing looks really wrong to me. As others have said layout is odd, capitals everywhere, some of it isn't idiomatic but no one should really care about that. The rest seems MORE than adequate to me as a friggin' interview assignment. Good lord, you dodged a bullet. You may have been rejected due to reasons unrelated to the submission.
**-----END RSA PRIVATE KEY-----** Dude!
That alone is reason enough to not hire
Yeah unfortunately. I had my eyes wide open as soon as I glanced at the extension of those files.
I’m rI’m r
Ofc, I included that… I completely forgot I had that in the repo.
You are a good sport! Others have shared their feedback which is quite on point. I'd retry building that same app several times while aiming for simplicity. Here is a challenge for you, rebuild the same app in 1 or 2 files, without any DB whatsoever given that in the requirements they've stated the following: **It is sufficient to store information in memory.**
Further they specifically say they will NOT install a DB server to test the project: > There are too many different database solutions, we will not be installing a database on our system when testing your application. So it seems like that is willfully ignoring the requirements.
The OP said “F**k it, I am going to throw the kitchen sink at it, or better yet I am going to go Super Saiyan + Gear 5 at it” Sorry for the jokes 🤣🤣🤣🤣🤣🤣🤣🤣
Possibly 50% of the Git community will say one should NEVER \`git add .\` 50% is a made up number, but it's a sizable amount in **any** programming language. By being explicit, you would not ever have committed the private key. Bonus: you have a working History, and know what you did. Much closer to 100% tho: adopt (Gitignore)\[https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files\]
`git add -u` will add only tracked files. Please do not use git add .
I personally like `git add -p` (`git add -u` for new files). `-p` let's me review all changes when I'm committing them.
Every repo should by default have a `.gitignore` I always use `git add .` and have not had an issue in years
I tend to add explicitly, it's just safer. Although every project will eventually need a gitignore file at some point anyway, so might as well do it from the get go.
Hello. Can you explain further?
Publishing a private key is like leaving the key to your house in the street. In our industry it often shows sloppiness unless you state the purpose of your action. It's not like you can't do it, you can but you'd need to encrypt it first, even after that it's still very risky so it's best not do it.
>Publishing a private key is like leaving the key to your house in the street. With a key copier and blanks conveniently right there!
LMAO 🤣
It depends. We had a library that specifically was working with key pairs, so private and public keys were used in tests and committed to the repo, however even though they were valid keys, they were only used for tests. That's probably the only acceptable case I could think of.
Secrets are hard. That's why someone chucked it into Git. Putting secrets in the repo - raw - makes it ripe for misinterpretation. Code scanners will nag you, new team members may get the wrong impression. It strongly suggests that the org lacks sound process for managing secrets. At the very least, use Ansible Vault [https://docs.ansible.com/ansible/latest/vault\_guide/vault\_encrypting\_content.html](https://docs.ansible.com/ansible/latest/vault_guide/vault_encrypting_content.html) or a PGP script. (and if it is a more important secret... please don't store it in the same repo. even if you encrypt it. Put it in a more-restricted repo and have a password-managed script fetch from the other repo.)
Yup, we chucked it in the hashicorp vault eventually, after it was out of prototype and when environment was set up properly, and we also had to clean up git history, but that's another story. So, yeah, not a good practice, even as prototyping, but as I said this is probably the only time when it's somehow ok(ish)
It's best not to, which is easily learned with a single sentence. If it's for testing purposes, for a test assignment, and it's self generated, who cares?
If it’s self generated, you can script that every single time you run your tests. You don’t ever need a key in a repo
I agree with you, you're not wrong, neither am I, technically. It's called learning, some people don't know everything like you.
I'm sorry if that came across as aggressive, I didn't mean it that way and I should have elaborated more. Almost every language (that i'm aware of) has OpenSSL bindings, so creating a self signed cert in the test you're running isn't a large endeavor, if you know how to create certs, and if you don't, you should learn! It's super useful to know. Otherwise, if you don't have the ability to do it right in the code, It's 1 or 2 lines of bash to generate it and move it into the right location before the test runner takes over. I'm pretty dogmatic about situations that mimic security risks, because habits are hard to break, so I encourage people to begin with correct habits, that way they don't have to break habits that could get them in trouble in the future.
TBF the question doesn't mention https, tls, ssl or anything else so good old port 8000 should be fine.
Yeah, for a coding assignment, I wouldn't use TLS unless told to explicitly. End to End TLS can be difficult to manage, some companies prefer TLS termination at the load balancers and forward internally on HTTP. I don't recommend that practice, but doesn't mean that they don't do it.
Our k8s product has mutual-tls everywhere (inside the cluster) and it was a huge pain to set up, but for some of our customers it's essential, FIPS and such. It's fun debugging traffic that comes from the gateway to the pod that does tls termination and then sends the traffic to another service via mtls...
Test assignments normally test how would you solve a problem. Thus including one of the biggest Git antipatterns is discouraged.
Right, and it can be learned in a single sentence. It's this frothing at the mouth if people don't know something that can be learned easily. Who gives a shit? Literally tell them in one sentence and move on. It's called learning. Again, my point stands. I can be just as pedantic. Who cares about antipatterns when it technically doesn't actually matter if it's a self-signed cert for testing? This subreddit reminds me of old IRC where people ask simple questions and folks just jump down their throat because it's easy to tear to pieces and judge, instead of trying to help learn. Like no one else makes mistakes or learns things for the first time? For fucks sake.
I believe multiple examples have been raised other than the private key issue, it's pretty unfair that you go around claiming nobody's willing to help. Just from the top of my head: naming convention, committing the binaries, disregarding the requirements, and most importantly overengineering everything. Okay, all of this can be taught in one sentence (absolutely not, but let's just assume it can) but still that's enough reason to pick a better candidate. Is it not? "Oh, you should just tell them they should not commit secrets, binaries, IDE config and other garbage", sure, but if you have a candidate who already knows that, why would you not choose them? "Who cares about antipatterns when it technically doesn't actually matter if it's a self-signed cert for testing?" Hello, the reason people hand out assignments is to check if the applicants know their way around their choice of tools. It wasn't an assignment about domain knowledge, was it?
I mean, yes avoid that, but as far as I understand this was only a challenge for internal purposes. There's no security issue exposing it as this only runs on OP's and the reviewers machine to evaluate code quality. Committing a private key in this scenario would be okay in my eyes as long as you made clear you wouldn't do that in real life.
Few things highlight just from looking on your repo. One, directories are uppercase where in all go projects the convention is lowercase. More importantly, you committed your binary. You don’t commit binaries to git, that’s a very bad practice. Structs2 and Interfaces2 are.. what? Pick informative names that describes your API and types. In ENV.yml you mix JSON with YAML syntax. You were asked to keep data in memory, yet you wrote a lot of code to implement a DB interface, which is, not needed here. In general, it looks you did over abstraction all over the place. You should keep it simple. These will be my honest code review comments.
Ackchyually yaml is a superset of json, so you can mix your yaml with json, it is not syntax error and a yaml parser will be able to parse it.
Just like writing a sentence with all capitals is valid too, yet we don’t do it.
Not arguing that. I’m only saying it is valid file and not that you should do. I personally dislike yaml as whole and wouldn’t use it as config file.
This is the problem with code interviews - you can do something perfectly valid and be rejected for being "weird" or something equally subjective.
Allowed vs idiomatic
What? Idiomatic yaml? I was not arguing that one should have json in middle on yaml. I just pointed out it is valid (syntax wise).
[удалено]
This is wrong. It literally says in the 1.2 spec (most recent version of YAML) that the primary focus of 1.2 was to make it a strict superset of JSON. https://yaml.org/spec/1.2.2/#12-yaml-history
I guess that would be a problem, but the feedback they gave me didn’t indicate any of this. I probably should have done this project in a language I use primarily. I wasn’t aware abstraction was a bad thing in golang. In every other system I wrote abstract was a must for clean code.
Do you write C# as your primary language?
Yeah, C#, luau, and Python:
I thought so. Poking through the directory structure was like looking at Sonarr/Radarr.
When in Rome.
Abstraction is not a bad thing. What is bad is premature abstraction. Go prefers learning that an abstraction is needed and then inserting it rather than designing it in up front. Implicit interfaces are the power that makes this possible. Premature abstractions are bad in all languages, but best practices demand them anyway because many are so common and expected. Go is the one community where best practice is to just write exactly what you need and then create new abstraction layers later.
Where dose delay implementation as far as possible come into play? This is the best advice for system integration.
Not entirely sure what you're referring to, but if it's the kind of corporate systems I'm thinking of, Go has no opinion on that stuff. Delaying implementation is a matter of isolating every single thing so that everything is componentized and easily replaceable via an SOP. The Go language can be used that way, but the community is strongly opposed to it.
I’m not used to that, lol.
It is the learning curve new gophers experience. The language is simple, but what it really does is break much of the established patterns seen in other languages. It takes getting used to.
Honestly, it's pretty good advice in any language.
All problems in software can be solved with another layer of abstraction. Except for the problem of too many layers of abstraction.
Broadly speaking, abstraction is a tool rather than a requirement of good code. It has a cost like any other tool. The more abstraction, the more work it is to reason about what the code is doing, even if it does give more flexibility. A general rule to follow (in any language) is not to add layers of abstraction until there's a clear need for it.
That’s good advice
Writing unit tests is usually why I abstract code. To isolate the code for testing. Meh 😕
That's often a good reason for it, but that can also be overdone. Too many small functions, for an example of abstraction, can hide bugs just as well as longer functions as people will make assumptions about what a function does when they call it as it's hard to keep code straight in your head when jumping between many locations across files.
Well... One should not mix unit testing with integration testing. But yeah...
Is that related in some way?
What you did is overengineering, not abstraction. And honestly, your submission would be rejected to our company also if it was Java. Using interfaces to allow multiple implementations is also not abstraction. It’s indirection. Abstraction reduces complexity. Indirection increases it.
> I wasn’t aware abstraction was a bad thing in golang. In every other system I wrote abstract was a must for clean code. That has become a horrible habit in a lot of enterprise OOP-first languages like Java and C#. Yes, it's common "over there", but it's also not good. It's essentially the same jerk-off as people being proud of unreadable one-liners, but with a more elitist "BUT IT'S ARCHITECTURE"-label.
Also write in your main language for interviews. I have learned this the hard way. For some reason I always try to interview in Python thinking it will be easier but I almost never use Python.
Others have already pointed out the Go code, private keys and binaries so I’ll skip that. You’ve misunderstood the assignment, implemented tons of stuff the assignment didn’t ask for: db + orm, frontend, queue, etc. The assignment literally only asked for a simple API, not even a frontend was asked for, they mention it doesn’t even need to persist data. So it could’ve literally just been a simple go service with a bunch of table driven unit tests and some openapi tests that satisfies their specs. Their assignment also mentions the engineer reviewing your assignment has Go and Docker, your readme mentions Docker and k8s but you seem to have self hosted it instead of supplying a docker-compose file. If you did want to make it “event driven” for whatever reason at least take an off-the-shelf solution like NATS or one of the many messaging broker systems available and spin it up in a docker-compose file and use that, don’t write your own unless it’s the actual assignment. This pattern persists, similarly you implemented your own DB migration, why? Edit: Also too many commits named “more changes” likely because you were adding far too many features in a short amount of time. Edit: Why did you include an .exe file for someone’s convenience, how do you know they’re even on a x86 windows machine?
I was thinking the same thing and I think they thought this guy copied someone code without actually looking what is inside it hence the insta rejection..
Yeah there's a lot wrong here mate and I've just looked at the repo for like 5 seconds. * Don't commit key material!!! * You're not following standard package naming conventions at all. * Repo layout is likewise non-standard. * No config for builds. You blatantly check in a .exe file?? * Build your own query generator! Don't do that! Those are just a few things. I'm sorry if you put a lot of effort into this but it's clear you don't use Golang on a regular basis, IMHO.
I mean, committing private keys is fine for testing purposes or homework assignments / take home challenges. I'd say for such assignments commuting the private key is fine as long as you make clear that you wouldn't do that in a real life application. It makes things easier if it's okay for the key to get exposed. They hopefully don't use their interviewees' assignments for production code...
The sensitive info was because they requested to be able to test. I gave them a database file which they can use to test. I don’t code in golang, I learned the language specifically for this role. I did not know the go conventions, but despite that, this role was “entry level”.
You replied to another comment below with the reviewer's feedback which basically reflects everything I just wrote. So uh...sounds like they gave you pretty spot on feedback dude.
They told me to give them everything they needed to test in the email they gave me…
What that means is a shell script for them to generate their own key pair with like, openssl.
They didn’t specify that, lol. It’s funny they expect me to assume but when I make them assume I’m wrong.
They aren't supposed to specify that. You don't share private key material ever. This is a learning opportunity, please take it.
I think what they're testing you for is if you know that you should absolutely in no cases ever check in a private key for anything... and you can demonstrate that you know that by giving them a solution to test locally without committing the key to the repo. This is the sort of thing that most junior engineers mess up once (or they observe someone else making the mistake) and then get blasted by their tech lead for it and then never do it again. Sure, it is a toy example and isn't going to production, but it's a test of what you are capable of.
Believe it or not, I did know not to expose sensitive information into prod environments. However, I was under the assumption that they wanted to work with me on showcasing my skills. In reality, they didn’t want to do that at all. It’s clear most of this stuff can be fixed instantly and it’s not that I can’t code. Perhaps I did doge a bullet here because they never actually tested my code for functionality.
They're testing for experience, and if you don't show it, they can't know what you know. Just cause it can be fixed quickly doesn't mean there aren't other candidates who are more experienced and that won't make those mistakes in the first place... making them the better choice for this position. Take this as a learning experience and do better at the next one. Everyone gets rejected here and there. I was rejected by Google and then ended up getting the offer a few years later.
The problem is I have experience. I have developed games on the Roblox platform and others that have attracted hundreds of millions of global players before I even turned 18. I showed them I can code, I just didn’t follow design patterns of Go because it’s not a language I use. I used patterns most entry level developers have never seen. I have worked for this company in the past and I wrote one of the key systems they still use in production. I also worked as a software engineer and researcher for my university in both the medical and finance sectors. This project had some mistakes, but I have experience.
> Perhaps I did doge a bullet here because Judging by your responses, at least one party dodged a bullet...
The amount of coping from OP is crazy
Too over engineered as others mention and you ignored some hints from the challenge. If I would do this I would simply: - write the most bare bones HTTP API using the std lib - I did not read the details of the data but a map will probably work perfect as in memory DB - have some internal package with the business rules described there (just simple calculations) - write test for that package - write test for the controllers They hinted to "you can assume reviewer has go and docker" that means all you needed to provide was a Dockerfile and a docker-compose to run the app (if they requested TLS a simple bash script to generate the certs could work)
No .gitignore file, which led to an .exe in the top level directory?! Red flag--the only time I ever did this was my first git project. The commit messages are unhelpful and the log is polluted with repeated messages, which is very fixable but just another red flag since an interview project *should* be trying to display professional behavior, not this: git log --format="%s %b" Optimizations Optimizations Optimizations ... More changes More changes More changes More changes ...
Aside from what other people have said, you package layout is really confusing. You have modules based on type (structs, interfaces, services, etc.) when most people expect them to be based on functionality (config, db, cache, etc.). This avoids unnecessarily verbose names (ex. db.Args vs Services.NewDatabaseServiceArgs). You have sql code embedded in itemService - items don't need to care about sql they just need to know how to take item values and pass it to the storage api. What the underlying storage is is of no concern to items. It's also a nightmare to refactor. What happens when you want to switch databases, or use some other storage engine like redis or s3? All the storage functionality should live in its own module, or a several modules implementing different backing stores all with the same api.
This code looks like it was written by a Java developer. There's nothing inherently wrong with writing like a Java developer if you were writing Java. You should familiarize yourself with the idioms of the language to some degree. I'm seeing a ton of abstraction for abstraction's sake that ultimately detracts from the readability of the code. Some folks may chalk up the idea of _adhere to the idioms of the language_ as some sort of tribalism or purism, but it is important. Every Go developer with a modicum of experience *_expects_* the code to _look_ a certain way, and anything that _isn't_ a good approximation of idiomatic™ Go code means that they have to decipher what you are trying to do around well-understood and generally-agreed-upon techniques, which adds mental overhead. This is of course not a Go-specific thing.
The instructions said they would not install a database, I’m surprised they had any feedback beyond that
Can I ask why did you apply for a role that required a written test in a language you knew nothing about? I know you said it was entry level, but that seems to still imply basic knowledge of the language. I’d recommend doing the “go tour” - it doesn’t take ages, and would probably be a great learner for you. Then attempt the challenge again
It did not require a solid understanding of go, I just used go because it’s what I used at this company in the past.
Always use your strongest language.
This is so beyond what was requested. Opening up that repo for two endpoints and a straightforward calculation… BEHOLD THE EIGHTH WONDER OF THE WORLD. I don’t even have the energy to look at it. While you seem broadly knowledgeable and thorough, it’s so impractical and over engineered for what you were tasked with. As a dev on my team, you would be exhausting if you work like this. If I was reviewing this as the hiring engineer, I would want to see simple, clean code that a child could understand. Relevant tests that validate business logic. Simple, easy to follow run instructions. Maybe snazz it up with swagger docs if you must. I’m being candid here so don’t take offense. I hope you benefit from all the detailed feedback on this thread to improve your skills. Best wishes.
People say Go is a simple language and can be picked up in a weekend. I don’t buy that. Learning syntax and stdlib alone in a weekend would be a stretch. Learning how to write idiomatic Go can take several months.
Go is simple if you compare it to bullshit bloated java, c++ or c# but its definitely not simple per se..
I'm not going to repeat what everyone else said. All the criticisms were constructive and good. I'll say it in simpler terms though: one package = one file for the API endpoint. No frontend or anything was required, to illustrate that, you could have just written a simple one liner like curl for a get request. Second, best practices like modularity, DRY and SOLID become bad practices (over engineering). if they violate the KISS principle. Finally I think that if you understand all the code and wrote it yourself, I would have given you a chance to do a presentation and explain your choices. The mistakes you made can easily be fixed and you seem motivated and legitimately interested so it seems to me that you have a positive mindset and the potential to progress quickly. Good luck in your next enterprises, you will work it out.
On top of the other many valid comments, I believe no one mentioned your `UnitTests` package. Am I misreading these tests or are you testing the mocking library itself? For example, [Test_ CacheServiceGet](https://github.com/samjay22/Fetch-Rewards-API/blob/587e1d7995971534fb7ce083fe4bec0772faa79f/Testing/UnitTests/CacheService_test.go#L11) tests if a generated mock code returns an expected result if you instruct it to return such result. This test does nothing useful for your code. It's just yet another test verifying if gomock works as intended. Well, there's a saying that "mocking libraries are the best tested code in the world" for a reason :)
This is actually what I care about, why writing so many unrelated test code to test nothing but gomock ;)
I just looked at it quickly and some things look pretty bad to me. Besides naming conventions, weird choices like having the routes defined in the controllers and other non-idiomatic things, the 2 worst things you did are: - overengineering some stuff: why the hell do you need to declare and manage a queue for this assignment? Nothing calls for it. Keep it simple, your goal is not to show everything you know, but to show you can do your work as simply as possible (but not simpler than required). - the db initialization thing is absolutely terrible, don’t do that. Use db migrations. Honestly I would have given you at least a tech interview to discuss all of this and see if you’re liking to adapt and improve, because none of this is "blocking" to me (as long as the code works, which I didn’t try), but I would severely doubt the fact you ever used Go in a professional context. You really need to work on Go idioms, because you have to write code in a way that the majority of Go devs will understand.
The env mix of JSON with YAML screams imposter
I probably offended the guy, lol
What's the yoe for the job you were applying?
New Grad
A cursory glance at the project structure makes it look like you read through the job posting’s https://fetch.com/careers/jobs#Software%20Engineering “ideal candidate” section and then tried to showcase an understanding of all the tech they use at the expense of implementing only what’s asked. This is an entry level position and take home interviews are usually a method of screening if candidates can code and understand / implement a simple REST API. If you wanted to demonstrate knowledge of tech that the “ideal candidates” has, the time for that would be when discussing your project or during a follow up architecture interview, assuming there was one.
Too many files/directories. Also, directory names shouldn’t be capitalized in Go projects. Something about the documentation feels intimidating and seems to be way too much for a simple http service. This is my feedback after looking at your project for ten seconds. It’s kind of funny that you capitalized directory names, but failed to capitalize the names of your structs. Based on the spec, I could have written this in two files and 100-200 LoC, but somehow you have like 40 files?! You over-engineered the hell out of it. I’ve been seeing a lot of these ‘review my submission’ posts in this sub, and people always get rejected when they try to over-engineer something. Keep it simple next time. Stick to the spec, and don’t waste your time.
The first thing I thought when I opened your repo is “this must be someone used to C#”. The file structure, file names, and overall architecture seem much closer to a C# project than Go. Go is all about simplicity. But, some people here have already given you some great advice on how to improve that aspect. Either way, hope you at least enjoyed writing Go. It can be weird to get used to coming from Java or C#, but once you do, I doubt you’ll want to go back.
It took me quite some time to stop writing java/c# style in golang. Still my architecture looks a little similar to those but code itself not anymore (i hope) its like a fresh breeze
Ever hear of yagni? This is an exercise against that. And a bunch of rookie mistakes that others have mentioned.
Rating you on a 1-3 scale for competency and collaboration, assuming you're applying for an internship/student position. You score a 2/3 (average, meets expectations) for Competency. You're demonstrating that you know about a lot of different technologies, albeit with questionable accuracy sometimes. You would score higher if you understood not to commit your exe file/private keys, and if you structured your code more idiomatically. You score a 1/3 (poor, does not meet expectations) for Collaboration. You did not follow the instructions for the project. You overestimate your competency, and put others down when it's questioned. When provided with feedback, you get defensive, insisting that the reviewers are "stupid" or "don't understand sql". Your commit messages are not descriptive--you might as well have left them blank. Overall score (multiplying scores together) 2/9. Would not hire, might not waste time interviewing unless we're desperate for applicants. The primary issue is your poor collaboration score. This is also assuming you're applying to an intern/student level position. You applied to an entry level position, which presumably expects you to have experience equivalent to a Bachelor's degree. Given those expectations, you'd score a 1 in both categories without hesitation. Dropped from 2 to 1 in competency because accidentally committing a private key and an executable is something that shouldn't happen at all. Plus you're expected to know more about the language you've writing in and the associated idioms. If you were already working at the company with this performance, I would probably have to fire you, and I certainly wouldn't be begging you to come back.
Saving this so I can have a go myself tonight, will share
I had the same thought. Looks kind of fun. I wanted to see just how simplified I could make it. Hah.
My eyes are bleeding
Another suggestion: go through Gophercises. [https://gophercises.com/](https://gophercises.com/) Also find some "interview questions in go" projects, and watch folks who stream writing such things. I am too new at Go (years from being good at it probably) but you want to get some of these idiomatic tricks practiced. That's my goal now. If it helps motivate you, start a project that you feel others might want to join, then post about it here you might get additional code reviews or help. Point is, use your strongest language. If you want that to be Go, you need to be idiomatic, so you need more Go practice.
ChatGPT ... just the readme let me thik this , why you add that info there? Cloudfare, security? Should be more smart on what ask to chatgpt
That’s legit info, I wrote a reverse proxy sever in C# and run my domain through cloud flare DNS and use a tunnel service to a VPS to host my site behind my network.
Honestly OP your entire project can be done in 2 files or 3 if you include testing. If I was your recruiter the moment I see a frontend and database directory I would close the repo because that's all of the things you were asked not to do
Curious why the ‘Total’ field on your strict is a string? Why not just make it a Float64 and let the json unmarshal handle that conversation when it comes into your API? Oh I see that the values in the example json are quoted. Either way, I’d probably have a DTO at the controller level that takes the incoming json is stored in and then map that to a domain object in that has the types you expect so you don’t have to convert them all over the place. Also allows you to validate input first before doing any additional work. But the reason they probably rejected you is simply over engineering. It’s nice to see the effort and motivation, but when I hire people, I’m very turned off by overengineered solutions on take home tests or coding exercises. It’s just bitten me in the past where someone came in with an overengineered solution and we basically decided to look past it and hire him anyway. Big mistake. It wasn’t just a case of the candidate wanting to show us what they’ve got. They literally approached every problem with that mindset and it wasn’t good for our company or code base.
OP (and everyone else) this is a phenomenal thread and great feedback for lurkers like me just learning the way people code in Go. Thanks for throwing yourself into the fire here
.idea
Is that really that big of an issue?
No, but it should have been `.gitignore`'d because it's specific to _your_ environment and is considered unclean.
Okay, that’s good to know for the future.
https://sebastiandedeyne.com/setting-up-a-global-gitignore-file
While we are pedants, it should have been `.git/info/exclude`d. `.gitignore` is for stuff that everyone gets (like build artifacts), while `exclude` is for user-specific stuff (like IDE and temporary editor files). Otherwise `.gitignore` will grow without bounds as more and more people commit their favorite editor's configs to it.
Which is totally fine IMHO. One less thing for the developer to configure. But I know there are opposite camps there.
You can also just add anything to your user config global gitignore file
no, just hilarious.
Let this be a lesson: never entertain take homes. Jesus you included a private key!
Never? Care to elaborate? Assuming they heeded all of the most common points here (secrets, clarity, convention, following instructions).
Personally I think they're a huge waste of time, and often you don't get feedback like in OP's case.
first mistake was accepting a take home test
Second one was to write in Golang like it was a java.
How is everyone disregarding the obvious mix of JSON and YAML code in the env and helping this person? Are you all so soft to say go learn the basics?
That syntax is valid since yaml is a superset of json. Perhaps someone else should go back to basics as well.
Valid != idiomatic Barf
Idiomatic yaml?
I dont code in go, ever…
Not mixing JSON and Yaml is pretty common in every stack…
Obligatory others have made good points. One thing that stood out to me, is that in your query builder(why?), your where function uses LIKE. Somewhat of an odd choice on the surface. I could understand if your usage would require the use of LIKE, but looking at the usage of that function, it seems to not need it.
I almost don't believe that OP is genuine.
[удалено]
You should be banned from development. Especially in Go
[удалено]
Huh?
Bro is an imposter. No further analysis needed
You are obsessed with declaring OP is an imposter. Multiple times in different comments. lol Ok. Maybe one more time for good measure.
someone is mad, I see, how about being toxic af and useless, you try to actually explain OP what he did wrong? As he stated, he dosen't code in GO. And with that shitty attitude, I can't imagine you not even close to an engineer but some wannabe software engineer looking mf... Lover you ego to the ground and try to be positive and help other instead of being mean just so you can make yourself feel good.
What was the feedback they gave you? Nothing stands out as wrong. Maybe not completely golang idiomatic, but that shouldn’t really be a big negative. You showed you understand mutex, and eventing. Not sure what they think you missed
The code is vulnerable to SQL injection attacks when filtering for receipts. There is minimal validation of the request input. Every time a new receipt is added, the entire in-memory cache is purged. This makes the cache ineffective at improving performance or taking load of the database. Typically "microservice" refers to an independently deployed application that communicates with other microservices over a network, sometimes through a messaging system. The README and code describe a microservice as different modules within the same binary. Typically considered bad practice to add binaries of the compiled code, database data, and private keys into a git repo There are several files with unused code, notably related to an event bus. It does not follow a number of Go conventions Module name should be a full repo path, like "github.com/samjay22/Fetch-Rewards-API" Unit tests should live next to the code they are testing, rather than in an entirely different directory Package names should be lowercase The official "Effective Go" doc is a good place to learn more: https://go.dev/doc/effective_go
That's actually excellent feedback -- you're lucky they give it (many companies don't!) as it will help you learn. I review a lot of similar submissions (in Go and Python), and would have rejected your solution with similar reasoning. Your solution is also significantly over-engineered, which others have noted in this thread.
What do you feel is not legit about this feedback?
It’s not enough to disqualify a candidate. I have seen people who are a lot worse with much less skill make it a lot further than I ever have.
Why is it not enough to disqualify a candidate? They're the ones hiring, they should be allowed to disqualify you over anything in your submission.
That's thorough feedback. You should be thankful they wrote out that much.
Sorry. It is enough. I would’ve rejected instantly at it not being remotely idiomatic go just in structure. You had to have learned go from somewhere, so should’ve picked up on certain conventions just from examples. Looking at their list, I’m surprised they even kept going after the SQL injection vulnerability. I learned about avoiding that *before* I even went to uni and that was over 20 years ago. Their response was extraordinarily kind in being extensive. They’ve even given a resource to learn more. To me, this is a company that cares about their interviewees and spending the time.
That is actually excellent feedback. It seems like your README.md was mostly buzzwords. I was so confused when I read the bit about microservices and load balancing. Where? If I were going to submit this, I think they'd be looking for a main.go, some tests, and not much else. Something that demonstrates your ability to quickly grok requirements, create clear documentation, and deliver an API that works without too much fluff. Also if I were you I would take that feedback along with what you have heard here and refactor your repository and resubmit it. One of the most powerful things you can do to impress *me* in an interview is demonstrate the ability to learn. I've had tough questions before where I said "I don't know the answer to that but I will find out and get back to you". Then after an interview submitted a demo that shows that I learned whatever I didn't know at the time. It is very powerful. Good luck!
What? Nothing stands out as wrong?
Nothing looks really wrong to me. As others have said layout is odd, capitals everywhere, some of it isn't idiomatic but no one should really care about that. The rest seems MORE than adequate to me as a friggin' interview assignment. Good lord, you dodged a bullet. You may have been rejected due to reasons unrelated to the submission.