6 ms·
If they're dissecting it at that close a level, it's probably not a company you'd want to work for anyway. At least how I judge take home tests (for a simple ba
by rhinoceraptor 7y ago
If they're dissecting it at that close a level, it's probably not a company you'd want to work for anyway. At least how I judge take home tests (for a simple backend CRUD app spec), I'm definitely not disqualifying based on surface level details like that. I don't even bother running it on my machine most of the time.
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.
- kamaal 7y agoThere are plenty of things in what you wrote. For example, My assignment was to build a command line app. The spec given said nothing about how bad inputs should be handled. How should they be handled? No Op? Throw exception? Friendly error messages? How? 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.
- tristanperry 7y agoI'm obviously not rhinoceraptor, but a lot of what they said seems correct to me (speaking as someone who's interviewed others a lot, but never used at-home code assignments as part of the process). _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 :)
- irahul 7y ago> In 24 hours? 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')
- jnwatson 7y agoDatabase migrations? Migrate from what? Most take homes are de novo, so have nothing to migrate from.
- swift532 7y agoDepends on what tech and approach you're using. Some people use migrations to create DB tables, for example. Or seed initial data like, say, what user roles exist.