-
Notifications
You must be signed in to change notification settings - Fork 4
Stages 2,3,4,5 #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,82 @@ | ||
| package org.hyperskill.database.clientside; | ||
|
|
||
| import com.beust.jcommander.JCommander; | ||
| import com.beust.jcommander.Parameter; | ||
| import com.google.gson.Gson; | ||
| import org.hyperskill.database.common.Request; | ||
| import org.hyperskill.database.common.Response; | ||
| import org.hyperskill.database.common.Util; | ||
|
|
||
| import java.io.DataInputStream; | ||
| import java.io.DataOutputStream; | ||
| import java.io.IOException; | ||
| import java.net.Socket; | ||
|
|
||
| public class Client { | ||
| public static void main(String[] args) { | ||
| System.out.println("Hello World!"); | ||
| @Parameter(names = {"-host", "-ip"}, description = "IP address") | ||
| String host; | ||
|
|
||
| @Parameter(names = {"-port"}, description = "Port", validateWith = PositiveInteger.class) | ||
| int port; | ||
|
|
||
| /* | ||
| @Parameter(names = "-t", description = "Command", validateWith = Commands.class) | ||
| String command; | ||
|
|
||
| @Parameter(names = "-m", description = "Text to save") | ||
| String text; | ||
|
|
||
| @Parameter(names = "-i", description = "Index") | ||
| String index; | ||
| */ | ||
|
|
||
| @Parameter(names = "-in", description = "Input file") | ||
| String inputFile; | ||
|
|
||
| @Parameter(names = "-out", description = "Output file") | ||
| String outputFile; | ||
|
|
||
| public static void main(String... argv) { | ||
| Client client = new Client(); | ||
| JCommander.newBuilder().addObject(client).build().parse(argv); | ||
|
|
||
| if (argv.length < 6) { | ||
| System.err.println( | ||
| "Usage: java -jar client.jar -ip <hostname> -port <port> -in <filename> [-out <filename>]"); | ||
| System.exit(1); | ||
| } | ||
|
|
||
| try ( | ||
| Socket socket = new Socket(client.host, client.port); | ||
| DataInputStream input = new DataInputStream(socket.getInputStream()); | ||
| DataOutputStream output = new DataOutputStream(socket.getOutputStream()) | ||
| ) { | ||
| System.out.println("ifile" + client.inputFile); | ||
|
|
||
| Request request = Util.read(client.inputFile); | ||
| Gson gson = new Gson(); | ||
| String out = gson.toJson(request); | ||
| output.writeUTF(out); | ||
| System.out.println("Sent: " + out); | ||
| String receivedMsg = input.readUTF(); // response message | ||
| System.out.println("Received: " + receivedMsg); | ||
| Response response = gson.fromJson(receivedMsg, Response.class); | ||
| if (response.isOk()) { | ||
| System.out.println("Request was successful"); | ||
| if (client.outputFile != null && request.getType()== Request.TYPE.GET){ | ||
| System.out.println("Value saved to " + client.outputFile); | ||
| Util.write(response, client.outputFile); | ||
| } | ||
| } else { | ||
| System.out.println("Request was not successful"); | ||
| } | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package org.hyperskill.database.clientside; | ||
|
|
||
| import com.beust.jcommander.IParameterValidator; | ||
| import com.beust.jcommander.ParameterException; | ||
|
|
||
| public class Commands implements IParameterValidator { | ||
| public void validate(String name, String value) | ||
| throws ParameterException { | ||
| value = value.trim().toLowerCase(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bad practice to reassign input parameter |
||
| if (!(value.equals("set") || value.equals("get") || value.equals("delete"))) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine, except for potential NPE, which I described in |
||
| throw new ParameterException("Parameter " + name + " should be set/get/delete (found " + value + ")"); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package org.hyperskill.database.clientside; | ||
|
|
||
| import com.beust.jcommander.IParameterValidator; | ||
| import com.beust.jcommander.ParameterException; | ||
|
|
||
| public class PositiveInteger implements IParameterValidator { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with naming as with |
||
| public void validate(String name, String value) | ||
| throws ParameterException { | ||
| int n = Integer.parseInt(value); | ||
| if (n < 0) { | ||
| throw new ParameterException("Parameter " + name + " should be positive (found " + value + ")"); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package org.hyperskill.database.common; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class Request { | ||
| public enum TYPE {SET, GET, DELETE} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now it's okay, cause you use it only inside the Also, enum is just another class, and so follows the same naming convention (upper camel case, instead of all-caps). The values of enum are constants, and so they are written in caps, but not the enum itself |
||
| private TYPE type; | ||
| private String key; | ||
| private String value; | ||
|
|
||
| public Request(String type, String key, String value) throws IllegalArgumentException { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use |
||
| Objects.requireNonNull(type); | ||
| Objects.requireNonNull(key); | ||
|
|
||
| this.type = validateCommand(type); | ||
|
|
||
| if (this.type == TYPE.SET && value == null) { | ||
| throw new IllegalArgumentException("Set command requires value, got null"); | ||
| } | ||
| this.key = key; | ||
| this.value = value; | ||
| } | ||
|
|
||
| public TYPE getType() { | ||
| return type; | ||
| } | ||
|
|
||
| public String getKey() { | ||
| return key; | ||
| } | ||
|
|
||
| public String getValue() { | ||
| return value; | ||
| } | ||
|
|
||
| public static TYPE validateCommand(String word) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not validation but parsing |
||
| switch (word.trim().toLowerCase()) { | ||
| case "set": | ||
| return TYPE.SET; | ||
| case "get": | ||
| return TYPE.GET; | ||
| case "delete": | ||
| return TYPE.DELETE; | ||
| default: | ||
| throw new IllegalArgumentException(word + " is not allowed command"); | ||
| } | ||
|
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package org.hyperskill.database.common; | ||
|
|
||
| public class Response { | ||
| public enum TYPE {OK, FAIL} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here as in |
||
|
|
||
| public boolean isOk() { | ||
| if (this.type == TYPE.OK) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, when your method returns boolean, you can just return boolean expression used in |
||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| public TYPE getType() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better have class parameters at the top of its definition. This way you and everyone else can clearly see what this class is constructed of |
||
| return type; | ||
| } | ||
|
|
||
| private TYPE type; | ||
|
|
||
| public String getValue() { | ||
| return value; | ||
| } | ||
|
|
||
| public String getReason() { | ||
| return reason; | ||
| } | ||
|
|
||
| private String value; | ||
| private String reason; | ||
|
|
||
| public Response(String type, String value, String reason) { | ||
| switch (type.trim().toLowerCase()) { | ||
| case "ok": | ||
| this.type = TYPE.OK; | ||
| break; | ||
| case "fail": | ||
| this.type = TYPE.FAIL; | ||
| break; | ||
| default: | ||
| throw new IllegalArgumentException(type + " is not allowed command"); | ||
| } | ||
|
|
||
| if (this.type == TYPE.FAIL && reason == null) { | ||
| throw new IllegalArgumentException("FAIL response requires reason, got null"); | ||
| } | ||
| this.reason = reason; | ||
| this.value = value; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package org.hyperskill.database.common; | ||
|
|
||
| import java.io.*; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.Paths; | ||
| import java.util.Objects; | ||
|
|
||
| public class Util { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating an "utils" class is tempting but actually is a bad practice. It tempts you to put unrelated stuff together into one class |
||
| public static Request read(String filename) throws IOException, IllegalArgumentException { | ||
| Path file = Paths.get(filename); | ||
| String command = null; | ||
| String key = null; | ||
| StringBuilder value = new StringBuilder(); | ||
| try (InputStream in = Files.newInputStream(file); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole thing could have been implemented in a couple of lines, using |
||
| BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) { | ||
| String line = null; | ||
| int lineProcessing = 0; | ||
| while ((line = reader.readLine()) != null) { | ||
| switch (lineProcessing) { | ||
| case 0: | ||
| command = line; | ||
| lineProcessing++; | ||
| break; | ||
| case 1: | ||
| key = line; | ||
| lineProcessing++; | ||
| break; | ||
| case 2: | ||
| value.append(line); | ||
| lineProcessing++; | ||
| break; | ||
| default: | ||
| value.append(" "); | ||
| value.append(line); | ||
| lineProcessing++; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| return new Request(command, key, value.toString()); | ||
| } | ||
|
|
||
| public static void write(Response response, String outFileName) { | ||
| Objects.requireNonNull(response, "Response can't be null in Util.write"); | ||
| Objects.requireNonNull(outFileName, "outFileName can't be null in Util.write"); | ||
| try (FileWriter fw = new FileWriter(outFileName, false); | ||
| BufferedWriter bw = new BufferedWriter(fw); | ||
| PrintWriter out = new PrintWriter(bw)) { | ||
| out.println(response.getType()); | ||
| out.println(response.getValue()); | ||
|
|
||
| } catch (IOException e) { | ||
| System.out.println(e.toString()); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package org.hyperskill.database.serverside; | ||
|
|
||
| import com.google.gson.JsonElement; | ||
| import org.hyperskill.database.common.Request; | ||
| import org.hyperskill.database.common.Response; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class JsonProtocol { | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better name it
CommandValidator, cause justCommandsis ambiguousThe name of a class should reflect on what it does