|
| 1 | +=== Ревью кода ==================== |
| 2 | + |
| 3 | +// Младший разработчик принес код нового сервиса на код ревью. |
| 4 | +// Надо сделать ревью кода сервиса DataService и метода создания/получения новой сущности Data. |
| 5 | +// Метод создания может вызваться из разных потоков и должен быть безопасен для такого режима использования. |
| 6 | +// AccessService и DataRepository безопасны для использования из разных потоков. |
| 7 | +// Предложить как можно сделать лучше и починить проблемы, если они есть. |
| 8 | +@ReqArgsContructor |
| 9 | +public class DataService { |
| 10 | + |
| 11 | + private AccessService access; // final, change name to `accessService` |
| 12 | + private DataRepository repository; // final |
| 13 | + private MessageDigest digest = get(); // should be removed, use Holder described below |
| 14 | + |
| 15 | + public DataService(AccessService access, DataRepository repository) { // could be deleted |
| 16 | + this.access = access; |
| 17 | + this.repository = repository; |
| 18 | + } |
| 19 | + |
| 20 | + public Data get(String uid) { // change name to findByUid |
| 21 | + access.checkRead(); // switch to use annotation-driven approach |
| 22 | + return repository.get(uid); // switch to findByUid() method or another |
| 23 | + } |
| 24 | + |
| 25 | + public void create(String name) { // return created object or creation status |
| 26 | + access.checkWrite(); // switch to use annotation-driven approach |
| 27 | + digest = get(); |
| 28 | + repository.save(new Data( // add return |
| 29 | + HexFormat.of().formatHex(digest.digest(name.getBytes())), // may be after switch to UUID - we could generate UUID by name OR use random UUID generation |
| 30 | + name |
| 31 | + )); |
| 32 | + } |
| 33 | + |
| 34 | + public static MessageDigest get() { // move into some MessageDigestHolder, pre-create and get already created object. add synchronized at least |
| 35 | + try { |
| 36 | + return MessageDigest.getInstance("md5"); |
| 37 | + } catch (NoSuchAlgorithmException e) { |
| 38 | + throw new RuntimeException(); // pass e as RuntimeException parameter, add error logging if needed |
| 39 | + } |
| 40 | + } |
| 41 | + |
| 42 | + public record Data( // extract to separate class |
| 43 | + String uid, // switch to use dedicated UUID class |
| 44 | + String name |
| 45 | + ) { |
| 46 | + } |
| 47 | + |
| 48 | + // @Repository to wrap exceptions |
| 49 | + public interface DataRepository { // extract to separate class |
| 50 | + void save(Data data); |
| 51 | + Data get(String uid); // change name to findByUid |
| 52 | + } |
| 53 | + |
| 54 | + public interface AccessService { // extract to separate class |
| 55 | + void checkRead(); |
| 56 | + void checkWrite(); |
| 57 | + } |
| 58 | +} |
0 commit comments