Update: The public links are down due to the company didn't wanted to have the assignment code shared. Sorry to say but the repo cannot be public anymore. Thanks for the everyone's feedback.
I have recently created a React.js project for one of the MNC I'm interviewing at and they provided a really detailed feedback of my assignment submission.
Sharing the codebase and the Feedback given by company here so anyone can benefit from it.
Requirement: Create a React.js app to list stocks and clicking on a stock will take to stock detail page which have stock quotes listing.
- Should be filtrable.
- Stock Quotes can be sorted by date.
- Once the Quotes expires fetch new quotes [polling].
- Should use
React.js.
Submission: (its private now)
- GitHub: Sensible Stock Portfolio
- Live Url: Sensible-Stocks
π Detailed Feedback
@Negatives
- code look messy and unnecessarily made complicated
- created a common http get service around axois, the implementation is not proper
- unnecessary component wrappers, just for using some CSS classes
- usage of error boundary is not in the recommended way
- unnecessarily added new functionality rather than concentrating on the complete solution
- unnecessarily suppressing lint rules
- not handling mobile responsiveness
@Instruments page / stock listing
- api call twice on page mount
- using 2 different state for search results and default view
- filter function will always runs
@Quotes page / stock detail page
- api call twice on page mount
- polling is implemented but the implementation is messy and won't work
implementation details
- spawn a web worker and listen for post message, inside this if the quotes list length is 0 make an api call to update the quotes
- providing the user a control to update the interval (1,2,3,5,10 ms) for checking expired quotes
- there's a set interval run on this interval (1..10 ms) inside this posting a web worker message
- web worker will run the loop for checking the quote's expiry and the expired leg is removed from the list
- once all items have been removed from the list, initiate the api call to fetch new quotes, this time frontend will hit the server at least 100-300 times (sometimes more than 1800 requests) reasons for this the web worker will receive a message every 1-10ms and hit the server
- not properly clearing set interval, leads to calling the previous apis as well, now the loop will hit the server more than 5k times
- Use string split and replace T and Z to convert the timezone for comparing the time
- timestamp not converted to IST in the table listing
@Positives
- Using typescript
- Using error boundaries
- segregating code by spliting components and util functions
Instruments page
- implemented search
Quotes page
- implemented sort
We thank you for the time, energy, and effort you invested in our process and we wish you the very best in your future endeavours.
Β
Disagree Points.
- What I didn't found helpful in this feedback is that it says the code looks messy, which I totally don't agree. Folks here can give their feedback about this and this will be super helpful.
- Also The react does render everything 2 times in dev mode in v18, that's why APIs get called two times, but in production it doesn't happen.
- The timestamp which is received by API must be in Unix timestamp or UTC to do date operations better, I had tough time doing that time conversion which I end up doing with
splitandreplace. Didn't wanted to use moment for this 1 task. - filter function will always run: what I'm suppose to do here, the filter function should run every time user type anything in the s search field.
- usage of error boundary is not in the recommended way - what is the recommended way ?
At the end It was a really nice experience and get to know new things.
Thank you for reading π€
Top comments (34)
Your code is a little bit messy.
Use this to learn, improve the quality.
Let me add some details on this,
There is extra stuff,
For a regular developer this is not expected, but for a senior position, this type of care is expected, in special if you are going to mentor other developers.
β« Second Point.
The default exporter of containers is created because there can be multiple exports of containers. [ex. - HOC for error boundary, HOC for Auth Provider and etc.] That's why it have only exports. But in case of a particular page there will be only page itself to be exported.
β« Third Point.
I accept it looks a little bit complex for just 2 routes, I could have just added two simple routes. But keeping the scalling of routes in mind I implemented it this way so It will be more easy to add childrens to the routes anytime in future.
Your feedback is really helpfull, what idea I get from the company feedback is that I should be doing what needed and in simply ways, instead of thinking the future prespectives. [deliver what needed π]
Thanks @shinspiegel
I was afraid that you get very pissed by my comment.
Thanks for understanding. :D
π
@shinspiegel hehe.. It looks like that when you start reading from top π€£π€£
not so sure this is right,
unforu
thank, this is sample of work i done, rate pleaser.
Thanks, I have learnt from your explanation as well
Yet another case of "demanding rampage" (I made up this term). Establishing the measuring stick far away from their daily work. This is such a good interview project :)
good work!
((I'd say it's actually better for you not entering a place where they have this interview process)).
Would you mind explaining why is it better not to enter such a place?
π€ I'll try!
The selection process is the very first contact the candidate has with the company culture. I mean the actual culture (cause every company has the most incredible values on the paper).
Got your point man, but what other options companies do have to access a candidate. I really like when companies ask me about my projects, challenges all other things and grind in details with tech rounds of logic building.
I also do not like doing take home assignments, It should only be in practice if anyone is hiring for fresher positions.
I got a 2x better opportunity than this π all that happens, happens for the good.
I recently passed this type of test for a company, in Ruby. It said it would take up to 4 hours but I ended taking 7 hours to complete it (the statement wasn't clear enough, and I made the test in TDD, which add more time than expected because I needed to consider what missing rule from the statement I should add or not).
Finally, they did not even took the time to make a feedback and I just had a vague explanation from the HR.
I was a bit vexed and angry, but anyway, I took that code to pass another type of test for another company: they want me to take some code and explain it live. It was really fresh in my mind and I could explain clearly all my choices. The CTO was really comprehensive and clear about the mistakes I made. Never say the least, this company hired me as a senior developer and tech lead. So, indeed, everything can be a valuable lesson!
agreed.
You left console.log in there. I'd block your pr if you tried to push that though (infact our company has dangerjs checks that do this for us)
They called it messy because:
If you're going to separate the components, start putting each component in it's own folder with the styles (use CSS modules).
You used a folder called containers, but it didn't contain any data layer or network layer just the error boundary.
That's all I noticed, I'm tired and about to head off to sleep.
If this was for a junior position, I'd hire you in a flash... You have promise - technically.
But then next step is - How you take criticism... It's more important than your perceived technical prowess.
If it was for a senior position then you've a long way to go before you start getting upset that you were rejected.
Thanks @airtonix It helpfull, I will try If i can optimize it.
And for consoles I personally use git pre-commit hook to block committing a code, but I didn't felt need to implement it here. But I should have removed them.
@airtonix if you can point out where is unnecessary use of promises exactly that will be really helpful.
For filter function runs everytimr or search methods you could use rxjs typeahead flow a very good example given here: learnrxjs.io/learn-rxjs/recipes/ty...
Rxjs can be a valuable addition to any js project
Thanks @krishnakantsalkar I will definately check it out.
I'm glad that you at least get feedback π
lol π π
Add debounce functionality for filter, figure out useDebuounce custom hook, this will be helpful in long term.
I have added a todo for that, I was about to di it but later decided to not do it because It wasn't making any much difference. But yes still good to implement.
Thank a lot for article.
Thank you for sharing this!
Thanks @yongchanghe
For your information
ReactJs render twice in development mode because of strict mode react component
Remove StrictMode component in index.ts then ReactJs will render only one time.