英文:

Refactoring - a code that puts the arrayList value of an entity that is similar or not the same

问题

目前，应用程序正在使用`Spring Boot 2.2`进行开发。

我感兴趣的重构部分位于用户实体上。

用户实体接收用户的喜爱工作和流派。

这个流派和工作包括用户实体，并且与每个实体具有`1:N`的结构，可以在没有重复的情况下进行多个选择。

```java
@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class User {

    @Id
    @Column(name = "user_id")
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    private Long id;

    private String userName;
    private String email;
    private String password;

    @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
    @JoinColumn(name = "user_id")
    private List<Job> likeJobs = new ArrayList<>();

    @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
    @JoinColumn(name = "user_id")
    private List<Genre> likeGenres = new ArrayList<>();
     
    ...

例如，流派包括'嘻哈、流行、韩流'，工作如'鼓手、DJ、Beatmaker和歌手'。

流派和工作结构本身可以视为相同。

因此，存在许多如下的重复代码。

public void addJobs(Job job){
        this.likeJobs.add(job);
        List<Job> jobsWithoutDuplicates = removeDuplicateFromJobs(this.likeJobs);
        this.likeJobs.clear();
        this.likeJobs.addAll(jobsWithoutDuplicates);
    }

    public void addJobs(List<Job> jobs){
        this.likeJobs.addAll(jobs);
        List<Job> jobsWithoutDuplicates = removeDuplicateFromJobs(this.likeJobs);
        this.likeJobs.clear();
        this.likeJobs.addAll(jobsWithoutDuplicates);
    }

    public void addGenres(Genre genre){
        this.likeGenres.add(genre);
        List<Genre> genresWithoutDuplicates = removeDuplicateFromGenres(this.likeGenres);
        this.likeGenres.clear();
        this.likeGenres.addAll(genresWithoutDuplicates);
    }

    public void addGenres(List<Genre> genres){
        this.likeGenres.addAll(genres);
        List<Genre> genresWithoutDuplicates = removeDuplicateFromGenres(this.likeGenres);
        this.likeGenres.clear();
        this.likeGenres.addAll(genresWithoutDuplicates);
    }

    public List<Job> removeDuplicateFromJobs(List<Job> jobs){
        return jobs.stream().distinct().collect(Collectors.toList());
    }

    public List<Genre> removeDuplicateFromGenres(List<Genre> genres){
        return genres.stream().distinct().collect(Collectors.toList());
    }

我认为我肯定可以进行重构，但我不知道该怎么做。

1. 重构后的代码必须是类型安全的。
2. 重构后的代码必须是线程安全的。
3. 重构后不得出现故障。

在满足条件的情况下，是否有办法进行良好的重构，而不违反面向对象编程的SOLID原则？

我首先尝试的方法是使用泛型类型。
我创建了addJobsOrGenres(List<?> JobsOrGenres)。

然后我创建了一个额外的方法叫做isInstanceOf()。

通过上述两种方法，工作和流派对象都可以进入任何方法进行处理，但我不知道这是否是一种美观的重构。

<details>
<summary>英文:</summary>
Currently, the application is being developed by utilizing `Spring Boot 2.2`.
The part I&#39;m curious about refactoring is located on the user entity.
User entities receive favorite jobs and genres from users.
This genre and job consist of user entity and `1:N` structure with each entity, and multiple choices are possible without duplication.
```java
@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class User {
@Id
@Column(name = &quot;user_id&quot;)
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
private String userName;
private String email;
private String password;
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
@JoinColumn(name = &quot;user_id&quot;)
private List&lt;Job&gt; likeJobs = new ArrayList&lt;&gt;();
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
@JoinColumn(name = &quot;user_id&quot;)
private List&lt;Genre&gt; likeGenres = new ArrayList&lt;&gt;();
...

For example, genres include 'Hip-hop, Pop, K-POP' and jobs such as 'Drummer, DJ, Beatmaker, and Singer'.

Genre and job structure itself can be considered the same.

Therefore, there are many duplicate codes as below.

public void addJobs(Job job){
        this.likeJobs.add(job);
        List<Job> jobsWithoutDuplicates = removeDuplicateFromJobs(this.likeJobs);
        this.likeJobs.clear();
        this.likeJobs.addAll(jobsWithoutDuplicates);
    }

    public void addJobs(List<Job> jobs){
        this.likeJobs.addAll(jobs);
        List<Job> jobsWithoutDuplicates = removeDuplicateFromJobs(this.likeJobs);
        this.likeJobs.clear();
        this.likeJobs.addAll(jobsWithoutDuplicates);
    }

    public void addGenres(Genre genre){
        this.likeGenres.add(genre);
        List<Genre> genresWithoutDuplicates = removeDuplicateFromGenres(this.likeGenres);
        this.likeGenres.clear();
        this.likeGenres.addAll(genresWithoutDuplicates);
    }

    public void addGenres(List<Genre> genres){
        this.likeGenres.addAll(genres);
        List<Genre> genresWithoutDuplicates = removeDuplicateFromGenres(this.likeGenres);
        this.likeGenres.clear();
        this.likeGenres.addAll(genresWithoutDuplicates);
    }

    public List<Job> removeDuplicateFromJobs(List<Job> jobs){
        return jobs.stream().distinct().collect(Collectors.toList());
    }

    public List<Genre> removeDuplicateFromGenres(List<Genre> genres){
        return genres.stream().distinct().collect(Collectors.toList());
    }

I think I can definitely refact this but I don't know what to do.

1. The refactored code must be type-safe.
2. The refactored code must be threaded safe.
3. Do not malfunction after refactoring.

Given the conditions, is there any way to do a good refactoring without violating OOP's SOLID principle?

The first way I did it was the generic type.
I created addJobsOrGenres(List<?> JobsOrGenres).

Then i created an additional method called isInstanceOf().

Through the above two methods, both job and genre objects processed methods that enter the whatever, but I don't know if this is a beautiful refactoring.

答案1

得分: 1

太长了，作为回答添加。

1. 如果您的addXXX方法使用Set而不是List，您可以摆脱removeDuplicateFromXXXX方法。如果您继续使用Set，请记得适当实现equals和hashcode方法。

2. 您可以摆脱addJobs(Job job)方法。只保留addJobs(Set<Job> jobs)方法。我认为这样做没有什么害处。这样，以后如果出现预处理或后处理逻辑，您只需要修改一个方法。对于addGenres也是同样的道理。

1. 重构后的代码必须是类型安全的。

当您使用List<Job>或List<Genere>时，类型安全已经得到保障。我不会选择addJobsOrGenres (List<?> JobsOrGenres)方法，如果出现针对job或genere的新要求，您将不得不添加更多的if-else语句。这使得更容易错误地将jobs误认为是genere，反之亦然。另外，参见上面的第2点，关于预处理和后处理，这是不应该这样做的另一个原因。

2. 重构后的代码必须是线程安全的。

您的代码对共享变量进行了修改，它不是线程安全的。您需要添加某种形式的锁定机制。根据您的用例（读取或写入次数多少），选择其中一种Lock策略。

英文:

Too long for a comment; adding as an answer.

1. If your addXXX methods take Set instead of List, you can get rid of removeDuplicateFromXXXX methods. Keep in mind proper implementation of equals and hashcode methods if you go ahead with Set.

2. You can get rid of addJobs(Job job). And let there be addJobs(Set<Job> jobs) only. I don't see a harm in that. This way you will have one method to modify in case a pre-processing or post-processing logic comes up in future. Same goes for addGenres.

> 1. The refactored code must be type-safe.

When you're doing List<Job> or List<Genere>, type-safety is taken care of. I wouldn't go with addJobsOrGenres (List<?> JobsOrGenres) - one new requirement comes for job or genere, you start adding more if-elses. This makes it more prone to mistake jobs for genere or vice-versa. Also, see point 2 above about pre and post processing as another reason why you shouldn't do this.

> 2. The refactored code must be threaded safe.

Your code does mutation of shared variables, it's not thread safe. You need to add a locking mechanism of some sort. Depending on your use-case (if there are many reads or writes), pick one of the Lock strategies.