Developer Interview

A technical interview is an almost indispensable attribute of the employment of any developer, and for senior developers, conducting them (interviews) is also almost an everyday duty. But how in a short time (ideally 20-30 minutes) to form a more or less acceptable idea of ​​the real experience of the applicant?

I remember the first time I was asked to talk to a young man on WPF - for several hours I made a list of what is worth asking (and double-checked the answers so as not to hit my face in the mud) so that we could at least say with some certainty what the company needs such a person or not. And now, armed with 10-15 questions, I enter the negotiation room, introduce myself, ask a couple of general questions and, among other things, clarify:
- And how many years of development experience using WPF do you have?
- I do not know WPF ....
- ...
This awkward moment when you understand that you have provided everything except the most obvious ...

Another, no less unexpected turn for me was when the applicant’s CV indicated 5 years of development experience and a bunch of intriguing project descriptions were listed, but in fact, a person with I was able to explain with difficulty how the reference types differ from the significant ones, but said about garbage collection that he only knows that you don’t need to think about memory in .Net ...

What can you ask, so that you can rely on the answer as something significant ?
- Ask what books he read and how many times? But the learned formulations do not mean that a person is able to solve real problems, the formulation of which differs from the book ones and sometimes requires deviation from the general principles of development for one reason or another.
- Write down all the technical nuances of the environment and ask about them? But in truth, who ever needed the knowledge of how garbage collection works (which also changes from version to version) and how many generations are there? I'm not saying that this extra knowledge is by no means, but knowledge or ignorance of this feature will not allow to determine the "quality" of the developer.
- Ask for a code example? But what code will they show you? How many people already rule it? In what conditions was it written? What if these brilliant 300 lines were written a month to the sound of the surf of the Atlantic Ocean in the rainy season? Can we then recreate the “working” atmosphere to get the next 300 brilliant lines?

I want to share my ideas and hear constructive criticism of this approach to interviewing. My idea is to show “OWN” code and listen. Over the evening, I sketched an example of a terrible code, including the most common “errors” in it. I expect that a senior developer with real development experience of 4 years or more should identify more than 80% of the errors and point out existing problems in a hypothetical architecture.

And so, actually the code:

1	using System;
2	using System.Collections.Generic;
3	 
4	namespace App.Services
5	{
6	    public enum LoginResult
7	    {
8	        Unknown = 0,
9	        Success = 1,
10	        WrongLogin = -1,
11	        WrongPass = -2,
12	        Error
13	    }
14	 
15	    public class LoginService
16	    {
17	        public string LastError = string.Empty;
18	 
19	        /// 

20	        /// Allow to login new user
21	        /// 
22	        /// login
23	        /// password
24	        /// asAdmin
25	        /// login result
26	        public LoginResult Login(string login, string password)
27	        {
28	            List dbLogins = new List();
29	            try
30	            {
31	                dbLogins.AddRange(
32	                    DAL.GetItems(
33	                    "select * from db.Login where Name='" + login + "'"));
34	            }
35	            catch (Exception ex)
36	            {
37	                lock ((object)777)
38	                {
39	                    LastError = ex.Message;
40	                }
41	                throw ex;
42	            }
43	            if (dbLogins.Count < 1)
44	            {
45	                return LoginResult.WrongLogin;
46	            }
47	 
48	            var prevUser = App.CurrentUser;
49	            App.CurrentUser = dbLogins[0];
50	            if (password.CompareTo(App.CurrentUser.Password) != 0)
51	            {
52	                App.CurrentUser = prevUser;
53	                return LoginResult.WrongPass;
54	            }
55	 
56	            var log = System.IO.File.AppendText(App.LogFile);
57	            log.WriteLine("New user loggined. Login=" + App.CurrentUser.Name);
58	 
59	            if (!(bool)((EventService)App.Service).SendWithConfirm(prevUser))
60	            {
61	                log.Write("Error sending to user.");
62	            }
63          
64	            GC.Collect();
65	            GC.Collect();
66
67	            return LoginResult.Success;
68	        }
69	    }
70	}

Errors (line number and description that I expect to hear from the applicant):
12 - The value for "Error" will be "-1", which will duplicate an existing one and will not allow distinguishing one from the other in the future.
17 (1) - Public field. According to the rules of good tone, it is not recommended to make fields publicly available.
17 (2) - Writing to a variable further by code is “implemented” through lock, but the external consumer may not be aware that the call to the variable should be synchronized with someone.
20 - Meaningless comments.
24 - Commentary is not true.
28 - The public method accepted parameters from outside, but did not check for their correctness (at least null).
32 - Strong connectedness.
33 (1) - Potential place to use SQL injection. Since concatenation and not parameters are used to form the request. And secondly, non-parameterized queries are not cached by the sequel server (if the DBMS is a sequel).
33 (2) - A similar style of query generation “binds” the application to a specific DBMS).
33 (3) - Concatenating strings in this way is not the most efficient solution.
35 - According to the rules of good form, you should catch errors that can be processed, and not everything in a row.
37 - This lock will not work.
39 - A meaningless value in a variable and no benefit to developers in diagnosing.
41 - The current stack trace, which could be useful in diagnosing an error, is irretrievably lost with such an exception.
49 (1) - Destruction of the application state, since if an error is lower (null password), the current user will already have a new login in the field.
49 (2) - Implicit business logic. Why cut off other logins?
50 (1) - The password is not checked for null and there may be an exception that will lead to the destruction of the application state.
50 (2) - The password is stored and used in clear text.
53 - According to the rules of good form - such detailed information about the causes of the authentication error should not be reported.
56 (1) - Lack of a well-thought-out logging mechanism as such.
56 (2) - A stream opens, but its closure and destruction were not performed, data may not be written to the final file.
56 (3) - The operation of working with files can raise an exception, which a level higher can be interpreted as an authentication error, although in fact the login has already passed.
57 - Meaningless information that will in no way simplify the life of a log file analyzer if it is suddenly created and whoever needs it.
59 (1) - In addition to being connected, nested calls with "overestimated" expectations regarding the return result.
59 (2) - Unobvious business logic.
61 - A meaningless log entry.
64.65 - A sign of a big memory problem in the application. (I also expect to hear why exactly two calls in a row and why you shouldn’t do this anyway)

Maybe someone has already taken this approach and can share their experience?