Enhanced the output of the NIOEchoClient Example #585
Conversation
Motivation: I was testing the NIOEchoClient example on Linux and noticed that I received no character output. I thought this would be handy and added this code to do so. Modifications: Small modification to the channelRead method in NIOEchoClient to read the bytes and transform to a string. Result: The display of the characters on your echo client before the channel closes.
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
Sources/NIOEchoClient/main.swift
Outdated
|
|
||
| while let byte: UInt8 = byteBuffer.readInteger() { | ||
| charArray.append(byte) | ||
| } |
There was a problem hiding this comment.
You can replace almost all of this with byteBuffer.readBytes(length: ByteBuffer.readableBytes).
Sources/NIOEchoClient/main.swift
Outdated
| let charString = String(bytes: charArray, encoding: .utf8) { | ||
| print("Received: '\(charString)' back from the server, closing channel.") | ||
| } else { | ||
| print("Received line back from the server, closing channel.") |
There was a problem hiding this comment.
I don’t think this else branch should be reachable.
Sources/NIOEchoClient/main.swift
Outdated
| @@ -12,6 +12,7 @@ | |||
| // | |||
| //===----------------------------------------------------------------------===// | |||
| import NIO | |||
| import Foundation | |||
There was a problem hiding this comment.
We shouldn’t need the Foundation import here, it should be possible to use the Swift standard library for the decoding. Reducing the number of imports in the examples is helpful for clarity, so it’d be good to do that.
There was a problem hiding this comment.
I used a loop converting bytes with UnicodeScalar. Let me know your thoughts.
Motivation: Improved the PR based upon feedback provided. Modifications: Cleaned up the code and removed Foundation. Result: A more concise example of my enhancement without Foundation.
Sources/NIOEchoClient/main.swift
Outdated
| var str = "" | ||
| for byte in charArray { | ||
| str += String(UnicodeScalar(byte)) | ||
| } |
There was a problem hiding this comment.
Ah, with this tighter loop it's a lot easier to see what we're getting into here. This can all be replaced with if let string = byteBuffer.readString(length: byteBuffer.readableBytes). We'll also want an error message for the non-bound branch.
There was a problem hiding this comment.
Thank you! Using if let string = byteBuffer.readString(length: byteBuffer.readableBytes) is a lot more concise than using the loop I have above.
Sources/NIOEchoClient/main.swift
Outdated
| public func channelRead(ctx: ChannelHandlerContext, data: NIOAny) { | ||
| numBytes -= self.unwrapInboundIn(data).readableBytes | ||
|
|
||
| var byteBuffer = self.unwrapInboundIn(data) as ByteBuffer |
There was a problem hiding this comment.
We don't need the as or the extra whitespace line.
Motivation: To make the PR easier to understand with less overhead. Modifications: Removed a whitespace, a cast, and a byte loop. Result: A more concise and easier to read PR.
Sources/NIOEchoClient/main.swift
Outdated
| @@ -21,13 +21,17 @@ private final class EchoHandler: ChannelInboundHandler { | |||
| public typealias OutboundOut = ByteBuffer | |||
| private var numBytes = 0 | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
I don't think we need this extra whitespace line either.
Sources/NIOEchoClient/main.swift
Outdated
| print("Received the line back from the server, closing channel") | ||
| if let string = byteBuffer.readString(length: byteBuffer.readableBytes) { | ||
| print("Received: '\(string)' back from the server, closing channel.") | ||
| } |
There was a problem hiding this comment.
I think we need a print clause in the implicit else branch here too.
Motivation: Making PR more concise. Modifications: Added else branch and removed whitespace. Result: A more concise PR to view character output.
|
@swift-nio-bot test this please |
Motivation: I was testing the NIOEchoClient example on Linux and noticed that I received no character output. I thought this would be handy and added this code to do so. Modifications: Small modification to the channelRead method in NIOEchoClient to read the bytes and transform to a string. Result: The display of the characters on your echo client before the channel closes. Motivation: Explain here the context, and why you're making that change. What is the problem you're trying to solve. Modifications: Describe the modifications you've done. Result: After your change, what will change.
Motivation:
I was testing the NIOEchoClient example on Linux and noticed that I received no character output.
I thought this would be handy and added this code to do so.
Modifications:
Small modification to the channelRead method in NIOEchoClient to read the bytes and transform to a string.
Result:
The display of the characters on your echo client before the channel closes.