patterncsharpMinor

Let's do some “enciph5r47g”

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

enciph5r47gletsome

Problem

I implemented the enciphering/deciphering algorithm from this code golf task.

Enciphering reads string from left to right, replacing each character

by a number N (0-9) to indicate that it is the same as the character
N+1 positions before it.

Only characters up to 10 positions back are

replaced.

Deciphering inverses the procedure.

An unciphered string to be enciphered must contain

characters in the range 32-126 and no digits (0-9).

An enciphered string must contain only characters in the range 32-126.

Cipher class

using System;
using System.Diagnostics;
using System.Linq;

namespace Enciph5r47g {
    public static class Cipher {
        public static string Encipher(string str) {
            CheckStringThrowException(str, IsEnciphered: false);

            string encStr = string.Empty;

            // For every character c in unciphered string...
            for (int index = 0; index= 0) && (digit A message describing the error in s, or empty string for no error.
        private static string CheckString(string s, bool IsEnciphered) {
            string stringDescr = IsEnciphered ? "Enciphered" : "Deciphered"; 

            char ch = s.ToCharArray().FirstOrDefault((c) => !IsAllowed(c));
            if (ch != char.MinValue)
                return string.Format("{0} string '{1}' contains unallowed character '{2}'", 
                    stringDescr, s, ch.ToString());

            if (!IsEnciphered) {
                ch = s.ToCharArray().FirstOrDefault((c) => char.IsDigit(c));
                if (ch != char.MinValue)
                    return string.Format("{0} string '{1}' contains digit '{2}'", 
                        stringDescr, s, ch.ToString());
            }

            return string.Empty;
        }

        private static bool IsAllowed(char c) => 32 <= c && c <= 126;

        #endregion
    }
}

Class CipherPair

```
namespace Enciph5r47g.Test {
internal struct CipherPair {
public CipherPair(string dec, string enc) {

Solution

Code style:

Normally you put braces on new line in C#.

Method parameters should use camel case: IsEnciphered => isEnciphered

You have inconsistent spacing between operators: backIndex >= 0 but index




Refer to this and this for other common guidelines.

char ch = s.ToCharArray().FirstOrDefault((c) => !IsAllowed(c));
        if (ch != char.MinValue)


This will probably fail if original string contains

char.MinValue

, producing a false-positive validation.

private static string CheckString(string s, bool IsEnciphered)


I would rather have two separate methods: one to check input and one to check output. Having a flag for that looks messy and scales poorly.

You can replace some of your loops with LINQ. For example, from what I can see this:

char digit = FirstDigit;
            bool digitIsUsed = false;

            // ...check its previous characters from right to left (so long as we are within available digits).
            for (int backIndex=index-1; (backIndex >= 0) && (digit <= LastDigit); backIndex--) {
                // If a character matches c, add a digit in place of c in encoded string.
                if (str[backIndex] == c) {
                    encStr = encStr + digit;
                    digitIsUsed = true;
                    break;
                }
                digit++;
            }

            // If no character matched c, add the original character in encoded string.
            if (!digitIsUsed)
                encStr = encStr + c;


is probably equivalent to this (IndexOf):

var digit = str.Reverse().Skip(str.Length - index).Take(10).IndexOf(c);
encStr += digit < 0 ? c.ToString() : digit.ToString();


I didn't have a chance to test it though ^^. This can be further optimized, if you reverse input string before entering outer loop. Then you can just use

str.Skip(index) instead of str.Reverse().Skip(str.Length - index). You can also replace outer loop with Enumerable.Select

. LINQ-based solution will not be as fast as loop-based one for obvious reasons, but it should be a lot shorter and probably better in terms of readability. 

The same thing can be done to decoding. 

Probably not that important, since you are code-golfing, but I think there is no reason for

Cipher

 to be static. Non-static class would be more useful. By that I mean that you can't really extend or swap out (in LSP-sense) a static class. If

Cipher` was a part of larger software I would recommend defining a contract, say:

interface ICipher 
{
    string Encipher(string str);
    string Decipher(string str);
}

and replacing

public static class Cipher

with

public class Cipher : ICipher

Then you would be able to extend or change encoding strategy implementation if needed without any hassle.

Code Snippets

char ch = s.ToCharArray().FirstOrDefault((c) => !IsAllowed(c));
        if (ch != char.MinValue)

private static string CheckString(string s, bool IsEnciphered)

char digit = FirstDigit;
            bool digitIsUsed = false;

            // ...check its previous characters from right to left (so long as we are within available digits).
            for (int backIndex=index-1; (backIndex >= 0) && (digit <= LastDigit); backIndex--) {
                // If a character matches c, add a digit in place of c in encoded string.
                if (str[backIndex] == c) {
                    encStr = encStr + digit;
                    digitIsUsed = true;
                    break;
                }
                digit++;
            }

            // If no character matched c, add the original character in encoded string.
            if (!digitIsUsed)
                encStr = encStr + c;

var digit = str.Reverse().Skip(str.Length - index).Take(10).IndexOf(c);
encStr += digit < 0 ? c.ToString() : digit.ToString();

interface ICipher 
{
    string Encipher(string str);
    string Decipher(string str);
}

Context

StackExchange Code Review Q#160274, answer score: 6

Revisions (0)

No revisions yet.