Skip to content

feat(core): YAML Parsing Logic #4

Closed
sung-jun98 wants to merge 7 commits intoapi-ghost:mainfrom
sung-jun98:yaml-read-login
Closed

feat(core): YAML Parsing Logic #4
sung-jun98 wants to merge 7 commits intoapi-ghost:mainfrom
sung-jun98:yaml-read-login

Conversation

@sung-jun98
Copy link
Copy Markdown
Collaborator

Related Issue

Description

  • Completed the logic to parse YAML files in the local directory and convert them to DTO.
  • DTO file definition complete
  • Add JUnit test code to check if the parsing result is reflected well
  • The logic to convert a local YAML file into a DTO file has been implemented.

Testing

image

  • I printed out whether the values ​​were entered correctly in the DTO class created based on the same content as the test file in Notion.
  • tested file

Additional Notes

Pre-Submission Checklist

  • Have you completed code style (convention) checks locally?
  • Have you passed the CI tests in your local environment?

@sung-jun98 sung-jun98 requested a review from haazz May 1, 2025 08:27
@sung-jun98 sung-jun98 changed the title Yaml read login feat(core): YAML Parsing Logic May 1, 2025
public class Position {

private Integer x;
private Integer y;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason you used the Integer wrapper class for the coordinate values?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think If the YAML is generated from the CLI, it could contain null values.

public class Position {

private Integer x;
private Integer y;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about using names like positionX and positionY instead of just x and y?

if (response != null) {
System.out.println(" response #" + (i + 1));

// when 출력
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove plz

@cod0216
Copy link
Copy Markdown
Member

cod0216 commented May 1, 2025

Other than that, everything looks great. Awesome work! LGTM 👍👍

/**
* HTTP methods (GET, POST .etc)
*/
private String method;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to manage it with enum.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could take a look at my code for reference—it might help!

/**
* HTTP or WEBSOCKET
*/
private String type;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to manage it with enum.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could take a look at my code for reference—it might help! ✌️

/**
* status of response ex. "200-299" or 200
*/
private Object status;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The object's type definition may benefit from improved type safety.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think String would be a better choice here.


/**
* 파일 경로로부터 시나리오 YAML을 파싱
*
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write comments in English.

System.out.println("\nSteps information:");
for (int i = 0; i < scenario.getSteps().size(); i++) {
Scenario.Step step = scenario.getSteps().get(i);
System.out.println("Step " + (i + 1) + ": " + step.getName());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that System.out.println is used quite frequently. Would using StringBuilder or another approach be more efficient?


// 각 스텝 정보 출력
System.out.println("\nSteps information:");
for (int i = 0; i < scenario.getSteps().size(); i++) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more efficient to avoid calling scenario.getSteps().size() on each iteration of the loop.

Scenario scenario = reader.readScenario(filePath);

System.out.println("===== 변환된 Scenario 객체 정보 =====");
System.out.println("===== converted Scenario information =====");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid using System.out.println in test logic! 🏆

}
}

// steps 출력 (null 체크)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the Kr-Comment??

ObjectMapper objectMapper = new ObjectMapper(new YAMLFactory());

File yamlFile = new File(yamlFilePath);
Scenario scenario = objectMapper.readValue(yamlFile, Scenario.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to handle an IOException here.

private String name;
private String description;
private String scenarioId;
private Integer timeoutMs;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use long type to represent microseconds.

@kobenlys
Copy link
Copy Markdown
Collaborator

kobenlys commented May 1, 2025

Appreciated the hardwork @sung-jun98
Stay strong and keep up the great work! 🦾🦾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants