D
D
Dmitry Bashinsky2018-09-19 13:18:46
ASP.NET
Dmitry Bashinsky, 2018-09-19 13:18:46

Can you rate the level of the code?

The other day I wrote a test task, I would like a couple more people to evaluate and give an assessment.
Tell us to improve or how would you do it differently?
The client code is automatically generated and does not need to be evaluated.
TK
Solution

Answer the question

In order to leave comments, you need to log in

3 answer(s)
A
ApeCoder, 2018-09-19
@ApeCoder

I would ask you to bring the tests to the form in which the requirements are in the TOR. For example, how to subscribe to an rss feed and then download its contents. Compliance check with items hit the feed. The tests simply check for null on the return value.
To be honest, I didn’t understand, maybe I overlooked where the rss conversion code is to the general view and what needs to be done to add another format.
There is a place an exception is caught and nothing is done with it

�
âš¡ Kotobotov âš¡, 2018-09-20
@angrySCV

the amount of work performed is impressive (there are not so many who want to work anywhere) -> accordingly, as a test task, you certainly did it successfully.
As for the improvements, there are many points - but they rest on approximately one thing - redundancy .
for example, a very large size of some classes, a very large number of wrappers, too many checks, a lot of unnecessary brackets, wrappers, and so on.
For example, you have many variables initialized to zero (perhaps this is such a standard in c$, I'm not very familiar)
At the same time, in many places there are getters without checking for zero, in values ​​initialized with zero. Of course there is this check in many places, and it creates unnecessary redundancy in the code, perhaps if you immediately initiated with some default values ​​(or did lazy initialization before anything is known about these variables), you could avoid and errors, and extra wrappers.
In general, the solution is poorly readable - BUT I repeat if this is a trial sketch - then this is quite a good option, and only with a long time finished products are born from sketches, the quality largely depends on the level of effort invested, it is stupid to expect that someone will put a month into the test task work)
I would definitely not recommend this to anyone.
P.S.
there is one subtlety in that those who do test tasks never pass them)
Normal developers already have all these typical sketches in various variations, they simply refer to what they have, a normal developer will not spend on such a useless task time, only juniors do this, and juniors are usually not needed.

B
basrach, 2018-09-22
@basrach

Punctuation and style are striking, you can argue like "I'm not a writer," but they meet by clothes, you know ...
By code.
Here you say DDD, but it looks like Transaction Script. Since in the FeedLoader class, in each non-query method, changes are saved, i.e. there is no concept of a business transaction. In this case, this is not particularly necessary, but since you stuttered about DDD ... Also, a rich isolated model is not visible. Perhaps in this case, again, this rich model is nowhere to take, because the subject area is trivial. But then it turns out that you have chosen the wrong way to solve the problem. In the Core module, you have some classes, obviously for serialization, it is not clear what they do in Core. And in general, if I saw the code without a description, I would never even have crept into the idea that it was created according to DDD.
The API is not REST, although the requirements were about REST.
There is no cache configuration, what, where and how much to cache. At the same time, caching itself is implemented in BusinessLayer, which is strange, since the cache, like, for example, DataAccess, is purely infrastructural things.
Logging only in the API and only errors.
There are no mocks in tests. And it seems that testing is only integration, i.e. without a database, you won't be able to test anything.
Empty catch in ApiTester. Yes, this is not the main code base, but when it does not work and at the same time does not tell why it does not work, it always infuriates, even if these are tests.
The final score depends on your expectations and the expectations of the reviewers. If you are a junior and applied for the same position, then it’s quite good. If senior, then it depends on the level of reviewers/project/company, etc.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question