Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/libraries/dart_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include: ../dart_test_base_browser.yaml
172 changes: 172 additions & 0 deletions examples/libraries/lib/convert/build_custom_codecs.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
import 'dart:convert';

// #docregion caesar-encoder-sink
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.

(For readability of the file itself, I'd put the CaesarCodec first, then CaesarConverter, and then the private classes they depend on. But that's just, like, my way of structuring things: Most important first, things it depends on below. If you don't need to know the details of the dependencies, you can just not scroll down, and if you need to find a detail, you only need to look below, not above.

If this file only exists as a file to extract chunks from for the article, then its readability probably isn't important.)

/// A [StringConversionSink] that applies a Caesar cipher [_shift] and
/// forwards the result to the [_output] sink.
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.

Single-line first paragraph.
Consider:

/// A [StringConversionSink] that applies a Casar cipher.
///
/// Shifts inputs by [_shift] and emits the shifted strings on
/// [_output].

class _CaesarEncoderSink extends StringConversionSinkBase {
final int _shift;
final StringConversionSink _output;

_CaesarEncoderSink(this._shift, this._output);

@override
void addSlice(String chunk, int start, int end, bool isLast) {
final buffer = StringBuffer();
for (var i = start; i < end; i++) {
buffer.writeCharCode(_shiftCodeUnit(chunk.codeUnitAt(i), _shift));
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.

(I'd have a String _shiftCodeUnits(String input, int shift) {...} helper. You write this code repeatedly. Well, at least twice.)

}
_output.add(buffer.toString());
if (isLast) {
_output.close();
}
}

@override
void close() => _output.close();
}
// #enddocregion caesar-encoder-sink

// #docregion basic-encoder, caesar-encoder
/// Encodes a string by shifting each letter forward in the alphabet.
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.

Noun phrases for classes.
Consider (making it one CaesarConverter class instead of both encoder and decoder):

/// Casear cipher converter that shifts letters forward by [shift].
///
/// Converts strings to strings by shifting every ASCII letter
/// forwards in the alphabet by [shift] positions, wrapping over
/// from `z` to `a` and `Z` to `A` if shifting past the end of the
/// alphabet.

// #docregion chunked-encoder
class CaesarEncoder extends Converter<String, String> {
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.

This class is basically repeated as CaesarDecoder, with a single 26 - shift as the only change.

If someone understands the Ceasar cipher, they'll know that encoding and decoding is the same operation with a different key.
If they don't, the exact example won't mean much.

Could have

final class CaesarConverter extends Converter<String, String> { 
  // ...
  const CaesarConverter(int shift) 
      : assert(0 <= shift && shift < 26, '$shift should be in the range 0..25'),
        _shift = shift;
  const CaesarConverter.inverted(int shift) 
      : assert(0 <= shift && shift < 26, '$shift should be in the range 0..25'),
        _shift = (26 - shift) % 26;
  // ...
 }`

(Uses %26 instead of .remainder(26) because the latter sadly isn't potentially constant. If the assert runs, the value will be in the 0..25 range, which ensures we won't have more than 26 constant instances. If assert doesn't run, and the converter is created from a runtime value, the stored shift can be negative, but using .remainder(26) in the actual conversion corrects for that.)

// #enddocregion chunked-encoder
/// The number of positions to shift each letter forward.
final int shift;

/// Creates an encoder that shifts each letter by [shift] positions.
const CaesarEncoder(this.shift);

@override
String convert(String input) {
final buffer = StringBuffer();
for (final codeUnit in input.codeUnits) {
buffer.writeCharCode(_shiftCodeUnit(codeUnit, shift));
}
return buffer.toString();
}
// #enddocregion basic-encoder

// #docregion chunked-encoder

@override
StringConversionSink startChunkedConversion(Sink<String> sink) {
// Wrap the output sink if it isn't already
// a StringConversionSink.
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.

Can this fit on one line? I find it abrupt to read the line-break.
Consider

    /// Make sink a StringConversionSink, if it isn't already one.

// ignore: close_sinks
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.

This ignore-comment is visible to readers of the article.
(build-custom-codecs.md line 276, and one in 298 too.)

I'd not put that ignore here. It's not something a reader of the example needs to see.
If the lint was in the recommended set, then everybody should be assumed to have to adhere to it, but it isn't.
Example code should not assume something that we don't assume for all users, that's just confusing.
(If the lint needs to be ignored here, then it's not a good enough lint. A sink that is returned is clearly for later use, and should not need to be closed here, but at the call-site.)

If the ignore is needed in this repo, make it an // ignore_for_file:, so the example code included in the article doesn't contain it.

final stringSink = sink is StringConversionSink
? sink
: StringConversionSink.from(sink);
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.

(Could, and IMO should, be:

if (sink is! StringConversionSink) {
  sink = StringConversionSink.from(sink);
}

The code here is probably copying existing code that predates Dart 2's flow-based promotion. We can do better today.)

return _CaesarEncoderSink(shift, stringSink);
}

// #docregion basic-encoder
}
// #enddocregion caesar-encoder, chunked-encoder

// #docregion shift-code-unit
/// Shifts the specified [codeUnit] by
/// [shift] positions in the alphabet.
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.

Single line first paragraph.

Looks like it should fit on one line.

///
/// Only shifts lowercase ASCII letters (a-z).
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.

(Can farily easily do upper case too, just create

  const a = 0x61;
  const z = 0x7a;
  const caseDelta = 0x20;
  final lowerCodeUnit = codeUnit | caseDelta;
  if (lowerCodeUnit >= a && lowerCodeUnit <= z) {
    final letterNumber = lowerCodeUnit - a;
    return codeUnit - letterNumber + (letterNumber + shift).remainder(26);
  }

Easier than having to explain why it doesn't do it.)

/// All other characters are returned unchanged.
int _shiftCodeUnit(int codeUnit, int shift) {
const a = 0x61;
const z = 0x7A;
if (codeUnit >= a && codeUnit <= z) {
return a + (codeUnit - a + shift) % 26;
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.

Use .remainder(26) instead of % 26.

Using .remainder(26) guarantees a result in the range 0..25.

There is no guard on the shift value, so it can potentially be negative, which can make (codeUnit - a + shift) negative and then %26 gives a negative result.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm coming back to this a few months later (thanks for the thorough review), and I wanted to confirm this suggestion.

https://api.dart.dev/dart-core/num/operator_modulo.html suggests for the % operator, "The sign of the returned value r is always positive". Doesn't that match what we want here? A bit of a contrast compared to many other languages like Java and JS though, so perhaps it's worth me explicitly calling out the behavior of %?

}
return codeUnit;
}
// #enddocregion basic-encoder, shift-code-unit

// #docregion basic-decoder
/// Decodes a Caesar-cipher-encoded string by
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.

(If keeping separate decoder, all the same comments apply as to the encoder.)

/// shifting each letter backward in the alphabet.
// #docregion chunked-decoder
class CaesarDecoder extends Converter<String, String> {
//#enddocregion chunked-decoder
/// The number of positions to shift each letter backward.
final int shift;

/// Creates a decoder that reverses a Caesar cipher with [shift] positions.
const CaesarDecoder(this.shift);

@override
String convert(String input) {
final buffer = StringBuffer();
for (final codeUnit in input.codeUnits) {
// Shift backward by shifting forward by (26 - shift).
buffer.writeCharCode(_shiftCodeUnit(codeUnit, 26 - shift));
}
return buffer.toString();
}
// #enddocregion basic-decoder

// #docregion chunked-decoder

@override
StringConversionSink startChunkedConversion(Sink<String> sink) {
// Wrap the output sink if it isn't already
// a StringConversionSink.
// ignore: close_sinks
final stringSink = sink is StringConversionSink
? sink
: StringConversionSink.from(sink);
return _CaesarEncoderSink(26 - shift, stringSink);
}

// #docregion basic-decoder
}
// #enddocregion basic-decoder, chunked-decoder

// #docregion caesar-codec
/// A codec that encodes and decodes strings using a
/// [Caesar cipher](https://wikipedia.org/wiki/Caesar_cipher).
class CaesarCodec extends Codec<String, String> {
/// The number of positions to shift each letter.
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.

(Can continue:

  ///
  /// Is shifted forwards for encoding and backwards for
  /// decoding.

final int shift;

/// Creates a Caesar cipher codec with the given [shift].
const CaesarCodec(this.shift);

/// Creates a [CaesarCodec] that uses ROT13 encoding.
const CaesarCodec.rot13() : shift = 13;
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.

(Consider making this a forwarding constructor:

  const CaesarCodec.rot13() : this(13);

I personally prefer when a class has only one initializing constructor, that means I can check that to see how fields are initialized and any restrictions that apply to them.

And, more importantly than my personal preferences, when Dart gets primary constructors, this class can be re-written as:

class CaesarCodec(final int shift) extends Codec<String, String> {
  //
  const CaesarCodec.rot13() : this(13);
  //
}

without needing any change to the redirecting constructor.
Be prepared for primary constructors. 😉 )


@override
CaesarEncoder get encoder => CaesarEncoder(shift);

@override
CaesarDecoder get decoder => CaesarDecoder(shift);
}
// #enddocregion caesar-codec

// #docregion use-encoder
void useEncoderExample() {
const encoder = CaesarEncoder(3);
print(encoder.convert('hello')); // khoor
print(encoder.convert('xyz')); // abc
}
// #enddocregion use-encoder

// #docregion use-codec
void useCodecExample() {
const cipher = CaesarCodec(3);

final encoded = cipher.encode('hello');
print(encoded); // khoor

final decoded = cipher.decode(encoded);
print(decoded); // hello

// The `inverted` getter returns a new codec that
// converts in the inverse direction of the original codec.
final inverted = cipher.inverted;
print(inverted.encode('khoor')); // hello
}
// #enddocregion use-codec

// #docregion top-level-instance
/// A [CaesarCodec] with the standard shift of 13 (ROT13).
const rot13 = CaesarCodec.rot13();
// #enddocregion top-level-instance
127 changes: 127 additions & 0 deletions examples/libraries/lib/convert/build_custom_codecs_complete.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import 'dart:convert';

/// A [StringConversionSink] that applies a Caesar cipher shift
/// and forwards the result to [_output].
class _CaesarEncoderSink extends StringConversionSinkBase {
final int _shift;
final StringConversionSink _output;

_CaesarEncoderSink(this._shift, this._output);

@override
void addSlice(String chunk, int start, int end, bool isLast) {
final buffer = StringBuffer();
for (var i = start; i < end; i++) {
buffer.writeCharCode(_shiftCodeUnit(chunk.codeUnitAt(i), _shift));
}
_output.add(buffer.toString());
if (isLast) {
_output.close();
}
}

@override
void close() => _output.close();
}

/// Encodes a string by shifting each letter forward in the alphabet.
class CaesarEncoder extends Converter<String, String> {
/// The number of positions to shift each letter forward.
final int shift;

/// Creates an encoder that shifts each letter by [shift] positions.
const CaesarEncoder(this.shift);

@override
String convert(String input) {
final buffer = StringBuffer();
for (final codeUnit in input.codeUnits) {
buffer.writeCharCode(_shiftCodeUnit(codeUnit, shift));
}
return buffer.toString();
}

@override
StringConversionSink startChunkedConversion(Sink<String> sink) {
// Wrap the output sink if it isn't already
// a StringConversionSink.
// ignore: close_sinks
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.

(I'd drop the ignore completely in this file. It's still not relevant to a reader. So maybe just disable the lint entirely for the repo. No big loss.)

final stringSink = sink is StringConversionSink
? sink
: StringConversionSink.from(sink);
return _CaesarEncoderSink(shift, stringSink);
}
}

/// Decodes a Caesar-cipher-encoded string
/// by shifting each letter backward in the alphabet.
class CaesarDecoder extends Converter<String, String> {
/// The number of positions to shift each letter backward.
final int shift;

/// Creates a decoder that reverses a Caesar cipher with [shift] positions.
const CaesarDecoder(this.shift);

@override
String convert(String input) {
final buffer = StringBuffer();
for (final codeUnit in input.codeUnits) {
buffer.writeCharCode(_shiftCodeUnit(codeUnit, 26 - shift));
}
return buffer.toString();
}

@override
StringConversionSink startChunkedConversion(Sink<String> sink) {
// Wrap the output sink if it isn't already
// a StringConversionSink.
// ignore: close_sinks
final stringSink = sink is StringConversionSink
? sink
: StringConversionSink.from(sink);
return _CaesarEncoderSink(26 - shift, stringSink);
}
}

/// A codec that encodes and decodes strings using a
/// [Caesar cipher](https://wikipedia.org/wiki/Caesar_cipher).
class CaesarCodec extends Codec<String, String> {
/// The number of positions to shift each letter.
final int shift;

/// Creates a Caesar cipher codec with the given [shift].
const CaesarCodec(this.shift);

/// Creates a [CaesarCodec] that uses ROT13 encoding.
const CaesarCodec.rot13() : shift = 13;

@override
CaesarEncoder get encoder => CaesarEncoder(shift);

@override
CaesarDecoder get decoder => CaesarDecoder(shift);
}

void main() {
const codec = CaesarCodec(3);

final encoded = codec.encode('the quick brown fox');
print(encoded); // wkh txlfn eurzq ira

final decoded = codec.decode(encoded);
print(decoded); // the quick brown fox
}

/// Shifts the specified [codeUnit] by
/// [shift] positions in the alphabet.
///
/// Only shifts lowercase ASCII letters (a-z).
/// All other characters are returned unchanged.
int _shiftCodeUnit(int codeUnit, int shift) {
const a = 0x61;
const z = 0x7A;
if (codeUnit >= a && codeUnit <= z) {
return a + (codeUnit - a + shift) % 26;
}
return codeUnit;
}
43 changes: 43 additions & 0 deletions examples/libraries/lib/convert/build_custom_codecs_stream.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// #docregion stream-transform, fuse-pipeline
import 'dart:convert';
import 'dart:io';

// #enddocregion stream-transform, fuse-pipeline

import 'build_custom_codecs.dart';

// #docregion stream-transform
void streamTransformExample() async {
const cipher = CaesarCodec(3);

// Encrypt a file as a stream.
final encrypted = File('message.txt')
.openRead() //
.transform(utf8.decoder)
.transform(cipher.encoder);

await for (final chunk in encrypted) {
stdout.write(chunk);
}
}
// #enddocregion stream-transform

// #docregion fuse-pipeline
void fusePipelineExample() async {
const cipher = CaesarCodec(3);

// Create a codec that encrypts, then compresses.
final encryptAndCompress = cipher.fuse(utf8).fuse(gzip);

// Write encrypted, compressed data.
final output = File('message.gz').openWrite();
output.add(encryptAndCompress.encode('Secret message.'));
await output.close();

// Read and decrypt.
final bytes = await File('message.gz').readAsBytes();
final decrypted = encryptAndCompress.decode(bytes);
print(decrypted); // Secret message.
}

// #enddocregion fuse-pipeline
Loading
Loading