Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Zaytsev_Artem#39

Open
zarp86 wants to merge 16 commits intomasterfrom
feature/ZaytsevArtem
Open

Zaytsev_Artem#39
zarp86 wants to merge 16 commits intomasterfrom
feature/ZaytsevArtem

Conversation

@zarp86
Copy link
Collaborator

@zarp86 zarp86 commented Jul 11, 2021

my first commit...again (after deleting my old branch, with the incorrect name and creating the new one with the valid name) + homework_1 + README.md

…rect name,and creating the new one with the valid name...) + homework_1 + README.md
@zarp86 zarp86 added the readyForReview Sign for Artem to take a look label Jul 11, 2021
@NikolaevArtem NikolaevArtem added HW_1 Good to go style Style fix needed and removed readyForReview Sign for Artem to take a look labels Jul 11, 2021
@NikolaevArtem NikolaevArtem changed the title feature/ZaytsevArtem_homework_1 Zaytsev_Artem Jul 11, 2021
@zarp86 zarp86 added readyForReview Sign for Artem to take a look HW_2 labels Jul 26, 2021
@zarp86 zarp86 requested a review from NikolaevArtem July 26, 2021 14:38
@NikolaevArtem
Copy link
Owner

Why did you add HW_2 label? It means I approved your homework, which I didn't

@zarp86
Copy link
Collaborator Author

zarp86 commented Jul 29, 2021

Why did you add HW_2 label? It means I approved your homework, which I didn't

Sorry, this is my fault(. I will try not to do like this anymore)

@NikolaevArtem
Copy link
Owner

Clean commit history, good!

Comment on lines +13 to +14
implementation 'junit:junit:4.12'
implementation 'junit:junit:4.12'
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure these are needed. You allowed junit in source code

import java.util.Scanner;


public class TrafficLight {
Copy link
Owner

Choose a reason for hiding this comment

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

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hope, that i fixed it.

public class PyramidPrinter {

public void run(){
Scanner scanner = new Scanner(System.in);
Copy link
Owner

Choose a reason for hiding this comment

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

Opt: you should open scanner in try block. What if you'll get an exception in sout?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

System.out.println("Only 1 non-negative integer is allowed as passed parameter! Please, try again");
return;
}
//adding finally block (28.07.2021)
Copy link
Owner

@NikolaevArtem NikolaevArtem Sep 2, 2021

Choose a reason for hiding this comment

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

Opt: These comments are unnessessary

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done (deleted).

try {
int base = scanner.nextInt();
if (base < 0) {
throw new Exception();
Copy link
Owner

Choose a reason for hiding this comment

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

Opt: better use specific exception from Runtime hierarchy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

if (base < 0) {
throw new Exception();
}
drawingThePyramid(base);
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
drawingThePyramid(base);
drawPyramid(base);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, done (renamed).

Comment on lines 30 to 33
if (base == 0){
System.out.println("");
return;
}
Copy link
Owner

@NikolaevArtem NikolaevArtem Sep 2, 2021

Choose a reason for hiding this comment

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

Opt: Is it needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You were right, it's no needed. Removed.

public static void main(String[] args) {
// initializeForTests();
new RandomCharsTable().run();
// isValidLastStringForEven("Even letters - ");
Copy link
Owner

Choose a reason for hiding this comment

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

All excessive comments/commented-out lines of code should be removed. Keep your code clean

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems that i fixed it.

String inString = scanner.nextLine();
String tempStr = "";
char c;
int cols =0; int rows =0;
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn;t look good

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

int cols =0; int rows =0;
String strategy = "";
int paramCount=0;
// parse of input procedure
Copy link
Owner

Choose a reason for hiding this comment

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

If you need a comment, means you can move it to a separate method

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done (another method was created).

// if everything valid with input - create and print result
int [][] matrix = createCharsTable(cols, rows);
printCharsTable(matrix, strategy);
}//end of run;
Copy link
Owner

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't understand what exactly wrong, but i refactored it)


import java.util.Scanner;

public class RandomCharsTable {
Copy link
Owner

Choose a reason for hiding this comment

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

Please fix namings, declarations, whitespaces, cleanup and simplify your code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hope, that i've done it.

@NikolaevArtem NikolaevArtem added bug Code fix needed and removed readyForReview Sign for Artem to take a look style Style fix needed labels Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Code fix needed HW_1 Good to go

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants