Developer Interview

A technical interview is almost an essential attribute of the employment of any developer, and for senior developers it is almost a daily duty to conduct them (interviews). But how in a short time (ideally 20-30 minutes) to get at a less acceptable idea of ​​the real experience of the applicant?

I remember the first time I was asked to interview a young man on WPF — I made a list of what I had to ask for (a few hours) (and rechecked the answers so as not to hit my face in the dirt) so that at least with some confidence I could say such a person or not. And now, having armed with 10-15 questions, I enter the negotiation room, I introduce myself, I ask a couple of general questions and by the way I clarify:
- And how many years of development experience do you have with WPF?
- I don't know WPF ...
- ...
This awkward moment when you realize that you have foreseen everything except the most obvious ...

Another turn that was no less unexpected for me was when the applicant’s CV showed 5 years of development experience and listed a bunch of intriguing project descriptions, and in fact, the person could hardly explain how the reference types differ from the meaningful ones, but he said about garbage collection all he knows is that there’s no 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 suggest that a person is capable of solving real problems, the formulation of which differs from the book and sometimes requires deviation from the general principles of development for one reason or another.
- Write out all the technical details of the environment and ask about them? But in truth, who ever knew 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 is too much knowledge - by no means, but knowledge or ignorance of this feature will not allow to determine the "quality" of the developer.
- Ask for sample code? But what code will they show you? How many people have it already rule? 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 such an approach to interviewing. My idea is to show your code and listen. During the evening, I sketched an example of a terrible code, including the most common "errors". I expect that a senior developer, with real development experience from 4 years, should identify more than 80% of errors and point out existing problems in a hypothetical architecture.

And so, the actual 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 /// <summary> 20 /// Allow to login new user 21 /// </summary> 22 /// <param name="login">login</param> 23 /// <param name="password">password</param> 24 /// <param name="asAdmin">asAdmin</param> 25 /// <returns>login result</returns> 26 public LoginResult Login(string login, string password) 27 { 28 List<Login> dbLogins = new List<Login>(); 29 try 30 { 31 dbLogins.AddRange( 32 DAL.GetItems<Login>( 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, which I expect to hear from the applicant):
12 - The value for “Error” will be “-1”, which will duplicate the already existing one and will not allow distinguishing one from another in the future.
17 (1) - Public field. According to the rules of good tone is not recommended to make the field publicly available.
17 (2) - Writing to a variable further by code is “implemented” through lock, but the external consumer may not be aware of the fact that access to the variable should be synchronized with someone.
20 - Senseless comments.
24 - Comment is not true.
28 - The public method took parameters from outside, but did not perform a check for their correctness (at least null).
32 - Strong connectedness.
33 (1) - Potential place for using SQL injection. As for the formation of the query is used concatenation and not parameters. And secondly, non-parameterized queries are not cached by the sequel by the server (if the sequel DBMS).
33 (2) - Such a query formation style “binds” the application to a specific DBMS).
33 (3) - String concatenation is thus not the most efficient solution.
35 - According to the rules of good tone should intercept errors that can be processed, and not everything.
37 - This lock will not work.
39 - Meaningless value in a variable and no benefit for developers in diagnosing.
41 - The actual stack treys, which could be useful in diagnosing an error, is irretrievably lost with this kind of exception.
49 (1) - Destruction of the application state, since if an error occurs below (null password), the current user will have a new login in the field.
49 (2) - Implicit business logic. Why are other logins cut off?
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 the clear.
53 - According to the rules of good tone - you should not report such detailed information about the causes of the authentication error.
56 (1) - Lack of a thoughtful logging mechanism as such.
56 (2) - The stream opens, but its closing and destruction are not executed, the data may not be written to the final file.
56 (3) - The operation of working with files can generate an exception, which can be interpreted as an authentication error a level higher, although in fact the login has already passed.
57 - Senseless information that does not simplify the life of analyzing a log file in any way, if it is suddenly created and who needs it.
59 (1) - In addition to connectedness, nested calls with "overestimated" expectations regarding the return result.
59 (2) - Unobvious business logic.
61 - Pointless logging.
64.65 - Sign of big memory problems in the application. (I also expect to hear why exactly two calls in a row and why it’s not worth doing this)

Maybe someone has already applied this approach and can share their experiences?