bulletsDodged += 1
What I'm looking for that it doesn't have obvious SQL injections (I've seen this a depressing number of times), it has some semblance of unit/automated tests, API validation, error handling, database migrations, good naming, and the code isn't all jammed in to one or two files.
Note how the evaluation changes from one person to other, you could be someone who expects an exception to be printed, I could be someone who expects an exception to be the wrong way, and would rather prefer a No Op.
As you yourself mentioned, SQL injection validations are important to you. Think of it from a candidate's perspective, you gave them a project with a certain set of features to finish in a deadline, and they need to finish it while holding a full time job. The candidate does the best they can to finish all the features. But you seem to have an invisible/opaque measurement in the form of SQL injection validation. The candidate might think you measure in terms of feature completion(Which would be correct, the project needs to work to be considered finished). The candidate could implement your spec 100% and yet get rejected.
>>it has some semblance of unit/automated tests, API validation, error handling, database migrations, good naming, and the code isn't all jammed in to one or two files.
In 24 hours?
What are you hiring for really? And why that kind of tearing hurry?
Having people to finish this kind of a high intensity ultramarathon coding drill under a crippling deadline. And then expect them to not just finish the assignment but also comply to your invisible quality checklist which no one knows but you.
For a change try taking such tests yourself. Like biggish projects, under super tough time limitations, and while you are at your current day job, with all the quality checklist evaluations attached. And then have it reviewed by other programmers.
You will see everyone has their own evaluation methods, and big feature projects delivered under heroic efforts, with tough deadlines tend to force you into some or the other quality compromise.
_Some form_ of error handling and validation should be demonstrated, because that's a standard part of any program.
Equally SQL injection is _always_ important. That's not a 'hidden' judgement criteria. It's a fundamental security issue that should be protected against.
Again, I'm not rhinoceraptor, but the gist of their post seems correct. If I was reviewing some code and it had no error handling, no validation, lots of god classes and methods and was susceptible to the most common form of web-based security flaw, that is probably a sign that the candidate isn't suitable for the position.
Good, constructive feedback should then be provided.
>> Note how the evaluation changes from one person to other, you could be someone who expects an exception to be printed, I could be someone who expects an exception to be the wrong way, and would rather prefer a No Op.
I agree completely. Any good interviewer/code reviewer should be fine with someone using exceptions even if they prefer no ops.
If an interviewer did complain loads about that (without specifying how bad input should be handled), they are a bad interviewer.
>> The candidate could implement your spec 100% and yet get rejected.
But adding "The application should not be simple to hack" probably isn't something a company should need to add to the spec?
>> In 24 hours? What are you hiring for really? And why that kind of tearing hurry?
24 hours? really? Yes I understand your frustration, that's no-where near enough (assuming you mean a company asks for the solution back the day after requesting it).
If it was me, I'd assume that such a company doesn't value work-life balance and pass on them :)
No, not in 24 hours. In about 2 hours. The laundry list of things - api validation, error handling, db migrations, good naming - it's pretty routine. A person writing production apis will have no trouble integrating all of it in a very short period of time. This is after all what we do everyday. If a candidate thinks not having sql injection in the code is an explicit requirement or is going to take a lot of time, then that is not the person for the job.
Here is a pretty simple flask implementation of your laundry list - api validation, db migration, no sql injections, no n+1 queries, good naming, swagger ui...
from flask import Flask, jsonify
from flask_sqlalchemy import SQLAlchemy
from flask_migrate import Migrate
from flask_apispec import use_kwargs, marshal_with, MethodResource, FlaskApiSpec
from marshmallow import Schema, fields, validate, ValidationError
from sqlalchemy.orm import joinedload
# setup
app = Flask(__name__)
app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///dev.db'
db = SQLAlchemy(app)
migrate = Migrate(app, db)
docs = FlaskApiSpec(app)
# Return validation errors as JSON
@app.errorhandler(422)
@app.errorhandler(400)
def handle_error(err):
headers = err.data.get("headers", None)
messages = err.data.get("messages", ["Invalid request."])
if headers:
return jsonify({"errors": messages}), err.code, headers
else:
return jsonify({"errors": messages}), err.code
# models
POST_TITLE_LENGTH_MAX = 80
POST_CONTENT_LENGTH_MAX = 5000
class Post(db.Model):
id = db.Column(db.Integer, primary_key=True)
title = db.Column(db.String(POST_TITLE_LENGTH_MAX), nullable=False)
content = db.Column(db.String(POST_CONTENT_LENGTH_MAX), nullable=False)
comments = db.relationship('Comment', back_populates='post')
COMMENTER_LENGTH_MAX = POST_TITLE_LENGTH_MAX
COMMENT_LENGTH_MAX = POST_CONTENT_LENGTH_MAX
class Comment(db.Model):
id = db.Column(db.Integer, primary_key=True)
commenter = db.Column(db.String(COMMENTER_LENGTH_MAX), nullable=False)
comment = db.Column(db.String(COMMENT_LENGTH_MAX), nullable=False)
post_id = db.Column(db.ForeignKey('post.id'))
post = db.relationship('Post', uselist=False, back_populates='comments')
# schemas
class CommentSchema(Schema):
id = fields.Int(required=True, dump_only=True)
commenter = fields.Str(
required=True, validate=validate.Length(max=COMMENTER_LENGTH_MAX))
comment = fields.Str(
required=True, validate=validate.Length(max=COMMENT_LENGTH_MAX))
class PostSchema(Schema):
id = fields.Int(required=True, dump_only=True)
title = fields.Str(required=True, validate=validate.Length(
max=POST_TITLE_LENGTH_MAX))
content = fields.Str(required=True, validate=validate.Length(
max=POST_CONTENT_LENGTH_MAX))
comments = fields.Nested(CommentSchema, many=True, dump_only=True)
# dal
def get_post_list():
return Post.query.all()
def get_post(post_id):
return Post.query.options(joinedload(Post.comments)).get(post_id)
def create_post(title, content):
post = Post(title=title, content=content)
db.session.add(post)
db.session.commit()
return post
def add_comment_to_post(post_id, commenter, comment):
comment = Comment(commenter=commenter, comment=comment, post_id=post_id)
db.session.add(comment)
db.session.commit()
return comment
# views
class PostListResource(MethodResource):
@marshal_with(PostSchema(many=True))
def get(self):
return get_post_list()
@marshal_with(PostSchema)
@use_kwargs(PostSchema)
def post(self, title, content):
return create_post(title, content)
class PostResource(MethodResource):
@marshal_with(PostSchema)
def get(self, post_id):
return get_post(post_id)
class PostCommentResource(MethodResource):
@marshal_with(CommentSchema)
@use_kwargs(CommentSchema)
def post(self, post_id, commenter, comment):
return add_comment_to_post(post_id, commenter, comment)
app.add_url_rule('/posts', view_func=PostListResource.as_view('post_list'))
docs.register(PostListResource, endpoint='post_list')
app.add_url_rule('/posts/<int:post_id>',
view_func=PostResource.as_view('post'))
docs.register(PostResource, endpoint='post')
app.add_url_rule('/posts/<int:post_id>/comments',
view_func=PostCommentResource.as_view('post_comment'))
docs.register(PostCommentResource, endpoint='post_comment')You submitted code which doesn't have things in my laundry list. Noticeably your db calls aren't surrounded by try/except. You have written no documentation. You didn't ship the code with git repo. No unit or functional test cases. I also expect these days one would use Python's typing library. Most of this helps in code maintainability. The fact that your assignment has none of this means you can't be hired to write production code ....
... See where this is going?
Despite you having written the code, someone with a different laundry list of quality checks can fail you.
There is no laundry list. There is an expectation that a senior engineer writes codes like a senior engineer.
> You submitted code which doesn't have things in my laundry list. Noticeably your db calls aren't surrounded by try/except. You have written no documentation. You didn't ship the code with git repo. No unit or functional test cases. I also expect these days one would use Python's typing library. Most of this helps in code maintainability. The fact that your assignment has none of this means you can't be hired to write production code ....
Not sure if you are really this dense or playing dense - the point of the comment was to show that your assumption about api validation and sql injection being explicit requirements or taking too much time is ridiculous. It' simple that it can be written in a comment in about 10 minutes, not the "24 hours" or whatever you claim it is going to take.
The very fact that you think sql injection is an explicit requirement or takes work will be an instant deal breaker for any position with the possible exception of fresh grad positions.
> ... See where this is going?
Yes, I do. You are arguing reductio ad absurdum to hide the fact that the things which you claimed unreasonable are in fact routine, and your time estimates are off by order of 10.
Followed immediately by.
>>There is an expectation that a senior engineer writes codes like a senior engineer.
You have no written list, but an imaginary checklist running in your brain about how a senior engineer writes code. Other engineers have theirs. I ran some of it and guess what, in that list absence of basic documentation, unit test cases, basic exception handling, code with types. Or even input validation for functions, like null checks is a no go.
Yet, if you submitted your feature complete project under a tough deadline to me, I'm not going to sit down and split hairs about absence of a favorite add-on feature of mine. That is what we are discussing here.
The code you posted in the comment above obviously doesn't have things like documentation, or validating function variables. Not even None checks. This obviously happens when some one attempts to write and submit code in minutes. When you write code this quickly, you focus on the exact feature demand at hand. Which in this case was SQL injection.
Other people got their feature set, in which several security add-on features would have been good to have but not in the time given.
>>You are arguing reductio ad absurdum to hide the fact that the things which you claimed unreasonable are in fact routine, and your time estimates are off by order of 10.
Oh obviously anyone who can do produce thousand(s) lines of code 24 hour project in 2.4 hours, is some totally different beast altogether.
There sure could exist such people who can produce >1000 lines of highly tested, security hardened code. I have yet to meet them though.
The closest I've seen is people who could write Lisp macros. But even those people wouldn't recommend writing code at the rate of 1000 lines per hour.
[1] There is a lot wrong with your "improvements". db calls aren't randomly placed in try/catch - that will be absurd. And the None checks aren't there because of something which you can very clearly see in the sample code but you also very clearly don't understand.
>>Do you have any production experience >>db calls aren't randomly placed in try/catch - that will be absurd.
Calls to databases go wrong all the time. Session objects expire, if you have cert based auths, they expire all the time. Especially if you have a largish microservices stack. Or you have locks on db objects, which you would run into. Or that network got flaky, or that just a certain type of resources like maxing out on db connections. Or that you are just trying to do write something to a database that is not allowed, like a duplicate primary key.
Never assume a database or any IO call for that matter will always go right.
Always check for every possible exception that could likely be thrown and handle it appropriately. Preferably each db interaction function should be atomic, and exception should be bubbled/raised/thrown to the application/feature main loop so that appropriate action like a retry or a roll back can be taken.
>>And the None checks aren't necessary because of something which you can very clearly see in the sample code but you also very clearly don't understand.
Pretty much any large codebase, that passes objects around should always do Null pointer checks. This is because several times resource heavy objects are initialized only on certain conditions, and if such objects are passed around they must be checked.
Is that why you made that absurd claim about sql injection being an explicit requirement? And that weird figure of 24 hours for handling sql injection, and api validation?
> Never assume a database or any IO call for that matter will always go right.
I said "db calls aren't randomly placed in try/catch - that will be absurd". Because they will be handled at app level to return uniform error messages. Now I am sure you will go on pretending that when you said "db calls aren't in try/catch", what you meant was db calls can throw an exception and app will handle it.
> Pretty much any large codebase, that passes objects around should always do Null pointer checks. This is because several times resource heavy objects are initialized only on certain conditions, and if such objects are passed around they must be checked.
What did I say about None checks not necessary because of something which is visible in the code? What do you think those marshmallow schemas and use_kwargs is for?
I've been on the other end of that, where we were explicit that the goal of the exercise was to solve problem X with technology Y so that we could gauge the applicant's skill with Y. And yet, we'd still get submissions that used Z because it was way better at solving X than Y was. Well, yeah, we know that. That's not what we were trying to assess.
I'm not saying you did that. It just gave me flashbacks to those situations.
It sounds like you got multiple solutions that used the best tool Z, but yet you reward people for being fine with using an inferior tool. That's not exactly the pool of applicants I'd want to bias my search towards.
Edit: I just want to add that I don't mean to sound harsh. And maybe it would help to understand what X, Y, and Z are in your case. Details do matter!
What the interviewer wants to know in this example is whether the author understands Python and file IO and data structures. Sure, you'd use the Unix pipeline in a production setting because it's better tested and faster. But in an interview context, that doesn't tell you whether the candidate is more likely to store item counts in a list or a dict, and that's what you're really trying to dig into.
This is the classic XY problem.
If your requirements are to hire some one that can write functional test cases against the Python's standard library. Just ask them to do that.
It's as simple as that.
Software world is huge. You will have learn, forget, relearn and learn new things all the time. You don't change programmers every time a new requirement on a new technology comes along. Which is why if some can write grep in Perl, they can also with a little yak shaving write grep in Python as well. Then in Golang, Java etc. And if some one wrote grep in Scheme, they can also write it in Clojure, or Common Lisp.
Programming languages of a particular style(C, Lisp based, ML family etc) don't change much in semantic features in their categories.
The point of a take home project is to see if a person can get things done.
If a base requirement is the thing is implemented with X, like wanting a table made of teak wood, and you give me a table that doesn't have any teak in it you've ignored half of my requirements. I don't care if you assessed my needs as being better met by oak and you made a fine table, I need to put a teak table somewhere and you completely failed at that task.
The point of a take home test that says build foo in Python so we can see it in Python is not "to see if a person can get things done," it's to see how a person gets things done in Python.
You're kind of right about it being the XY problem, except it's where the misdirected candidate is trying to solve "how do I do thing X" when the real problem is "write a simple program in Y so we can know if you know Y". We aim to be very explicit about the project's requirements so that none of this is a surprise afterward.
Besides grep itself was built on work done by other people before.
https://lists.freebsd.org/pipermail/freebsd-current/2010-Aug...
GNU grep uses the well-known Boyer-Moore algorithm, which looks first for the final letter of the target string, and uses a lookup table to tell it how far ahead it can skip in the input whenever it finds a non-matching character.
If the idea here is to test if the candidate can come with the Boyer-Moore algorithm in an hour then I doubt something like that is even possible. Things like that are not invented by some one in an hour, the original inventors themselves likely spent a life time studying and working on problems to produce anything novel.
On the other hand memorizing other people's work, which they themselves took years work in discovery, in the current of Google look-up-able knowledge, literally proves nothing. It's the exact opposite of a person capability to produce things.
The whole idea of a take home test is to demonstrate capability. If submitting a working project under a tough deadline doesn't pass that test, then Im not sure what the hiring panels are even trying to do here.
Original:
cat error.log | grep 'ERROR:' | wc -l
cat error.log | grep -v 'SENSITIVE' | grep 'ERROR:' | wc -l
Original:
grep error.log 'ERROR:' | wc -l
grep -v 'SENSITIVE' error.log | grep 'ERROR:' | wc -l
gzip file.js | wc -c # oops, we've just removed file.js
cat file.js | gzip | wc -c # shows compressed sizePerhaps real programmers would 'grep -c 'ERROR:' error.log'. I’m not sure what kind of overhead the extra pipes use.
Looks like this is a common pattern with take home projects. I faced a very similar problem a few weeks back.
Though they called me for the interview. The general approach is to take some ones overnight deadline coding and code review it for a week. Then they dissect your code written in deadline duress like they are some masters of the universe. They also told me my code passed through their automated test suite, and then some programmers in their office reviewed it too.
And then after two rounds of interviews they rejected me because apparently 'cat error.log | grep 'ERROR:' | wc -l' is not how real programmers find error count, but write python programs every single time they face such a problem.
In short these places with take home interviews generally tend to be people who are full of themselves. They are also a kind of workplace full self-congratulatory people who have fallen in love with their own legend.
Haven't had a single good experience with places that give take home tests.