Property file into configuration in Go My first ever attempt at Go. Writing a small app. It takes a property file and uses it to populate a Config object.
After adding the first property of the Config object (dateformat) I realised that the code was looking horrible and would only get worse as I added more properties.

Please help me simplify, how should this be done? Why is my go-code always looking this horrible? :)

// Config holds the properties and values used for configuration
type Config struct {
    dateFormat string
}

// DefaultConfig is the set of configuration values used if none other is given.
var DefaultConfig = Config{"2006-01-02"}

// FromFile returns the configuration to use for did
func FromFile(path string) (c Config, e error) {
    c = DefaultConfig
    if lines, err := File2lines(path); err == nil {
        for _, line := range lines {
            prop, val, propErr := propertyOf(line)
            if propErr == nil && prop == "dateformat" && ValidDateFormat(val) {
                format, formatErr := ToGoDateFormat(val)
                if formatErr == nil {
                    c.dateFormat = format
                }
            } else if propErr != nil {
                e = propErr
            }
        }
    } else if err != nil {
        e = err
    }
    return
}

func propertyOf(str string) (key, val string, err error) {
    split := strings.Split(strings.TrimSpace(strings.TrimRight(str, "\n")), "=")
    if len(split) == 2 {
        return split[0], split[1], nil
    }
    return "", "", fmt.Errorf(fmt.Sprintf("String '%s' cannot be read as property.", str))
}

The code expect a file which has the following property:

dateformat=yyyymmdd

ValidDateFormat just checks if the given value is valid.
ToGoDateFormat converts any given format to valid golang date format
File2lines reads a file and returns its lines

OK, there's a couple of comments I can make WRT code style looking at what you have now. Things that really stand out, for example, are lines like this:

return "", "", fmt.Errorf(fmt.Sprintf("String '%s' cannot be read as property.", str))

The function in the fmt package is called Errorf, where the f at the end stands for format. Why, then bother with the fmt.Sprintf call, just to get a string? fmt.Errorf does the same job as fmt.Sprintf, only it returns an error:

return "", "", fmt.Errorf("string '%s' can't be read as property", str)

Golang coding standards also states that error strings should start with a lower-case character (see here)

Other, general comments - as I usually do, I'll go through your code pretty much line by line, commenting on things as I see them

var DefaultConfig = Config{"2006-01-02"}

Now this is a perfectly valid statement. However, it makes code less readable (especially if the declaration of Config sits in a different file, or contains a ton of fields. Imagine you need ~10 config values, some of them have defaults, some of them are not. To this end, I'd recommend you explicitly mention which field you're assigning, rather than relying on the order of the fields never changing:

var DefaultConfig = Config{
    dateFormat: "2006-01-02",
}

I also don't get why DefaultConfig (a variable) is exposed, even though the field it contains isn't. The default variable clearly is an internal thing, and should as such not be exported.
The same logic applies to the Config type: if it's exported, then surely there should be some fields or methods that are exported, too. Your snippet shows no such thing.

// FromFile returns the configuration to use for did
func FromFile(path string) (c Config, e error) {

I'd opt to return a pointer, here. It forces people to check for errors. Currently, you're returning a valid Config value, which can be safely used. I can call conf, _ := FromFile("/tmp/does_not_exist"), and assume everything went fine. I'm guaranteed to receive a valid value after all. Not so if I were to change the function to:

func FromFile(path string) (*Config, error) {}

If there's an error, I'll get a nil value, accessing that will result in a runtime panic, so I'm forced to defend against that, and check errors. I'll rewrite some of the code to use a pointer return value

// c = DefaultConfig <-- this is now gone
if lines, err := File2lines(path); err == nil {

Yes, again there's a comment I can make. because your if clause is true on success, the rest of your code is essentially wrapped in a big if. Golang programmers prefer to return early, especially if something goes wrong. Rewrite this to:

// don't know why this function is exposed, don't know what this func looks like, but I prefer CamelCasing (or camelCasing) the name - also recommended style
lines, err := FileToLines(path); err != nil {
    return nil, err
}
// one less level of indentation here

Now a bigger one. Your propertyOf function has a triple return value, again this is perfectly valid. However, it's clear that you're returning a key-value pair, and an error. Why not create a type kv struct{ key, value string} or something so it's clear that the first 2 strings belong together. This is safer because it doesn't allow for people being silly and ignoring one of the return values. The key means nothing without the value and vice-versa after all. The data looks better when grouped. Anyway, let's rewrite the body of your FromFile func now:

// not in a func
type kv struct {
    key, value string
}

conf := DefaultConfig
// in func FromFile
for _, ln := range lines {
    kv, err := propertyOf(ln) // we will look at propertyOf later
    if err != nil {
        return nil, err // again, return early
    }

    // valid
    switch kv.key {
    case "dateformat":
        // maybe check for empty strings and return errors here
        // you could even try parsing a known date, to see if the format is valid
        conf.dateFormat = kv.value
    case "some-other-key":
        // implement other values
    default:
        // optional, unknown config could just as well be skipped
        return nil, fmt.Errorf("unknown config %s (value %s)", kv.key, kv.value)
    }
}
return &conf, nil // all good

Now let's look at the propertyOf function. I must say, the name is confusing to say the least. I'd call it something like lineToKVPair or something. Then again, naming things isn't my speciality. However, when I see propertyOf, I'm expecting either a pointer to a field to be returned (through reflection), a boolean (whether or not an argument of type interface{} is a struct, and has a given property, or simply a bool given a value, telling me whether or not that value is a property of a given object.

// again: return pointer to ensure the error is checked
func toKVPair(ln string) (*kv, error) {
    parts := strings.Split(
        strings.Trim(ln, " \t\n"), // removes all spaces, tabs, and new-line characters from string in one call
        "=",
    )
    if len(parts) != 2 {
        return nil, fmt.Errorf("string %s could not be parsed to a key-value pair", ln)
    }
    return &kv{
        key: parts[0],
        value: parts[1],
    }, nil
}

Now putting all of this together, the code looks something like this:

type Config struct {
    dateFormat string
}

type kv struct {
    key, value string
}

var DefaultConfig = Config{
    dateFormat: "2006-02-01",
}

func FromFile(path string) (*Config, error) {
    lines, err := FileToLines(path)
    if err != nil {
        return nil, err
    }

    conf := DefaultConfig

    for _, ln := range lines {
        kv, err := propertyOf(ln)
        if err != nil {
            return nil, err
        }

        switch kv.key {
        case "dateformat":
            conf.dateFormat = kv.value
        case "some-other-key":
            // implement other values
        default:
            return nil, fmt.Errorf("unknown config %s (value %s)", kv.key, kv.value)
        }
    }
    return &conf, nil // all good
}

func toKVPair(ln string) (*kv, error) {
    parts := strings.Split(
        strings.Trim(ln, " \t\n"), // removes all spaces, tabs, and new-line characters from string in one call
        "=",
    )
    if len(parts) != 2 {
        return nil, fmt.Errorf("string %s could not be parsed to a key-value pair", ln)
    }
    return &kv{
        key: parts[0],
        value: parts[1],
    }, nil
}

This, to my eye, looks a lot tidier, and simpler. But it's still not what I'd actually do. My preferred approach is even simpler, still.

Use environment/config packages.

Golang projects are more often than not either CLI tools, or containerised binaries. In case of the latter, they usually get their config from environment variables. There's a lot of packages out there that handle this really nicely.

Now for some shameless self-promotion: I usually use this package, and it served me well. So much so that I've contributed a bit to it. To do what you're trying to do is dead-easy:

import(
    "github.com/caarlos0/env"
)

type Config struct {
    DateFormat string `env:"DATE_FORMAT" envDefault:"2006-01-02"`
}

func GetConfig() (*Config, error) {
    var c Config
    if err := env.Parse(&c); err != nil {
        return nil, err
    }
    return &c, nil
}

That's it! if you want some validation to be added, you can have a look at the custom parsers, or just add them on to the Config type:

// do validation here, and call that in the GetConfig func before you return
func (c Config) validate() error {}

When you are working on a CLI tool, there's other packages you can use that handle both flags and environment variables. The most commonly used one (in my experience) is viper. It's quite nicely documented, although I find using it can lead to quite verbose code in the end.

What you can quite easily do, however, is use the standard flags package, and use os.Setenv to populate the environment variables accordingly, and then use the code above:

import(
    "flags"
    "fmt"
    "os"
)

func main() {
    // create the flag
    flagSet := map[string]string{
        "DATE_FORMAT": "2006-01-02",
    }
    flags.StringVar(&flagSet["DATE_FORMAT"], "date-format", flagSet["DATE_FORMAT"], "set different date format string")
    // parse the flags:
    if err := flags.Parse(); err != nil {
        fmt.Printf("error: %+v", err)
        os.Exit(1)
    }
    // populate env
    for k, v := range flagSet {
        if err := os.Setenv(k, v); err != nil {
            fmt.Printf("error: %+v", err)
            os.Exit(1)
        }
    }
    // load config struct from the env vars we just set
    conf, err := GetConfig()
    // etc...
}

Note: All the code I wrote, I just typed here on the site, I've not tested anything, so there might be some typo's and such, but the basic ideas are all there. I think the code is close to complete enough to be used as a basis to write your own solution.

Have fun


Simplifying parsing command-line arguments I'm currently working on a command-line "database migration" utility. Here are some of the requirements regarding reading the command-line arguments part:

It should accept regular database connection parameters: host, database name, port, user and password (do not allow to specify password as a command-line argument, ask for password separately)
It works in two modes - "validate" and "apply" - one of them has to be specified explicitly, both cannot be specified
There should be an integer "batch size" argument specified
There can be an optional range of integer ids specified via --id-begin and --id-end (the beginning of the range defaults to 1 if not specified)
Instead of a range, there can also be a --name argument specified
Both name and id range cannot be present
There should be a "verbose" flag

My Solution

Here is what I came up with:

I'm using argparse module and extracted the parsing command-line arguments into a separate .parse_args() function which returns the parsed arguments (which is argparse.Namespace instance):

import argparse
import getpass

def parse_arguments(args):
    """Parses command-line arguments."""
    parser = argparse.ArgumentParser()

    # general database connection settings
    parser.add_argument('-H', '--host', help="Database host IP address", required=True)
    parser.add_argument('-D', '--database', help="Database name", required=True)
    parser.add_argument('-P', '--port', help="Database port", required=True, type=int)

    parser.add_argument('-u', '--user', help='Database user name', required=True)

    # modes
    parser.add_argument('--validate', help='Enables "validate only" mode', action='store_true')
    parser.add_argument('--apply', help='Enables "apply" mode', action='store_true')

    # extra settings
    parser.add_argument('-b', '--batch-size', help='Batch size - determines how many cases are processed at a time',
                        type=int, default=5000)
    parser.add_argument('--id-begin', help='ID for the processing to begin with', type=int, default=None)
    parser.add_argument('--id-end', help='ID for the processing to end with', type=int, default=None)
    parser.add_argument('--name', help='NAME value', type=str, default=None)

    parser.add_argument('--verbose', help='Enables "verbose" reporting mode', action='store_true', default=False)

    args = parser.parse_args(args)

    # if both specified, id end cannot be smaller than id begin
    if args.id_begin and args.id_end and args.id_end < args.id_begin:
        raise parser.error("Invalid ID range.")

    # at least one mode should be specified
    if not args.validate and not args.apply:
        raise parser.error('Please specify one of the "validate" or "apply" modes.')

    # don't allow to use both "apply" and "validate" mode
    if args.apply and args.validate:
        raise parser.error('Cannot use both "validate" and "apply" modes.')

    # don't allow to use both NAME and ID
    if args.name and (args.id_begin or args.id_end):
        raise parser.error("Cannot use both NAME and ID.")

    # set the default id_begin value
    if not args.name and not args.id_begin:
        args.id_begin = 1

    args.password = getpass.getpass(prompt='Enter password: ')

    return args

The Questions

The code works and I've even covered it with tests (put them into a gist). But, I don't particularly like the way I check the requirements for name and id ranges, mutually exclusive "apply" and "validate" modes. It feels like I could have used more argparse specific features like custom types or actions. What do you think?

I would also appreciate other ideas about handling the password - using getpass() helps to not have the password shown on the terminal explicitly, but requires mocking it in tests.

Your function should have args=None as default. This way you can use it to parse the command-line arguments as well as pass in a list of arguments to parse.

For your mutual exclusive options you should use...mutual exclusive groups. However, this only works properly for your mode, because adding a group of not mutually exclusive options (your --id-begin and --id-end) to a mutually exclusive group seems to no be supported at the moment, even though there was some activity on that a few years back: http://bugs.python.org/issue10984

# modes
action = parser.add_mutually_exclusive_group(required=True)
action.add_argument(
    '--validate', help='Enables "validate only" mode', action='store_true')
action.add_argument(
    '--apply', help='Enables "apply" mode', action='store_true')

With the above code, you can remove all checking you currently do on the modes. It requires that exactly one of the modes is activated.

Also note that the default behavior of action='store_true' is to have default=False, so you can just do:

parser.add_argument('--verbose', help='Enables "verbose" reporting mode',
                    action='store_true')


Prime factorization of an integer I have just started learning functional programming (Haskell) for fun after using imperative languages my whole life. I am looking for quick and simple tips from pros on how the particular code I just wrote (a simple program solving a simple task - factoring an integer) can be simplified and made more beautiful.

-- FactorInt: Factoring n to prime integers

import Data.Maybe

-- Takes the head maybe
headMaybe :: [t] -> Maybe t
headMaybe [] = Nothing
headMaybe (x:_) = Just x

-- Takes the first available divizor
takeDivizor :: Int -> Maybe Int
takeDivizor n = headMaybe $ filter (\i -> n `mod` i == 0) $ takeWhile (\i -> i^2 <= n) [2..]

-- Takes the maximum power of k in n
maxPower :: Int -> Int -> Int
maxPower n k
    | (n `mod` k) /= 0   = 0
    | otherwise          = 1 + maxPower (n `div` k) k

-- Factors an integer to a list of divizors
factorInt :: Int -> [(Int, Int)]
factorInt 1 = []
factorInt n = case (takeDivizor n) of
    Nothing -> [(n, 1)]
    Just a  -> (a, k) : factorInt (n `div` a^k) where k = maxPower n a

-- Pretty printing powers
prettyPowers :: (Int, Int) -> String
prettyPowers (a, 1) = show a
prettyPowers (a, b) = (show a) ++ "^" ++ (show b)

-- Pretty printing factors
prettyFactors :: [(Int, Int)] -> String
prettyFactors factors = foldl1 (\ a b -> a ++ "*" ++ b) $ map prettyPowers factors

-- Program
main = do
    nStr <- getLine
    let n = read nStr :: Int
    putStrLn $ prettyFactors $ factorInt n

Your code all looks clear and readable to me. This is basically all performance-related.

It's "divisor"

since you're not dealing with negatives, you can use the more efficient rem and quot instead of the more mathematically correct mod and div.
factorInt 1 should be [(1,1)].

In your prettyFactors method you're creating a bunch of left-nested ++'s. This leads to a quadratic complexity printing method. You can use foldr1 to switch them to right-nested ++'s and get linear time, or you could look into the showS trick. (this won't really matter since by the time your printing takes a noticeable amount of time the factorization will be taking ages.)
Addendum: In most cases foldl and foldl1 are not what you want. You usually use foldr or foldr1 when you want laziness and foldl' or foldl1' when you want strictness.

Every time you use factorInt you start takeDivizor at 2, despite the fact that you might have already tried 2,3,4.. on previous iterations. The simplest way to remedy this would be to just add a parameter to each of them telling them where to start looking for factors.

You check a lot of numbers you don't have to in your takeDivizor function, you really only need to look at a list of primes, not [2..]

You could modify maxPower to make it tail-recursive.

If I were to write this, I would probably leave out the headMaybe and takeDivizor and write factorInt something like this.

primes = 2 : [ p | p <- [3,5..], all (\x -> rem p x /= 0) (takeWhile (\x -> x^2 <= p) primes ]

factorInt :: Int -> [(Int,Int)]
factorInt 1 = [(1,1)]
factorInt n = f n primes where
  f n (p:ps) | n < x     = []
             | otherwise = let mp = maxPower n p in (p, mp) : f (quot n (p^mp)) ps

Note that my implementation of primes is far from optimal, you can find better implementations on the haskell wiki page for primes.


Create a C style char** from a C++ vector I am dealing with some older C style APIs, like Posix execve that take a char**.  In the rest of my code, I prefer to use a fairly modern C++ style, so I have vector of std::string.  Is this the best way to handle the glue between them?

char ** strlist(std::vector<std::string> &input) {
    char** result = new char*[input.size()];
    result[input.size()] = nullptr;
    for (int i=0; i<input.size(); i++) {
        char *temp = new char[input[i].size()];
        strcpy(temp, input[i].c_str());
        result[i] = temp;
    }
    return result;
}

bool del_strlist(char ** strings, int limit=1024) {
    bool finished = false;
    for (int i=0; i<limit; i++) {
        char *temp = strings[i];
        if (temp == nullptr) {
            finished = true;
            break;
        }
        delete temp;
    }
    delete strings;
    return !finished;
}

Since you already have a std::vector<std::string>, it's much simpler to let that own the memory, and build a parallel std::vector<char*> which just keeps pointers into the original strings. The only difficulty is ensuring this isn't used after the owning vector goes out of scope.

The simplest implementation is something like:

std::vector<char*> strlist(std::vector<std::string> &input) {
    std::vector<char*> result;

    // remember the nullptr terminator
    result.reserve(input.size()+1);

    std::transform(begin(input), end(input),
                   std::back_inserter(result),
                   [](std::string &s) { return s.data(); }
                  );
    result.push_back(nullptr);
    return result;
}

where the caller is responsible for managing the lifetimes of the two vectors. You get the required char ** by calling result.data().

If you want to avoid the potential lifetime problems, you can write a class to own both vectors for you so their lifetimes are synchronized.

Note that I deliberately haven't taken the vector by const ref, because execve takes an array of pointers to non-const char, but it doesn't mutate the contents so there's no need to make a copy. This lack of const-safety is common in old C interfaces.

PS. I didn't force myself to use transform when a simple loop would do hoping for some magic optimization. I used transform instead of a simple loop because it more clearly expressed the semantics of my code.

A "simple" loop is a powerful multipurpose language construct which could do anything. By comparison transform is a simple, special-purpose algorithm, and much easier to reason about as a consequence.

Not least, since I had to take a non-const reference to the argument despite having no intention of mutating it, anything that makes it easier to see at a glance that no mutation is possible, is helpful.


Basic arithmetic calculator in JavaScript Task is basically:

"Write a function which takes two number as the first two parameter. The third parameter is one of the arithmetic operators (+ , - , * , /). Execute the arithmetic operation (number1 operator number2) and return the result of it."

Here's the first idea I've had:

function calculate(num1, num2, operator) {
  num1 = parseFloat(num1);

  if (isNaN(num1)) {
    throw Error('First parameter invalid. Must be a number.');
  }

  num2 = parseFloat(num2);

  if (isNaN(num2)) {
    throw Error('Second parameter invalid. Must be a number.');
  }

  operator = operator.trim();

  if (!/^\+$|^\-$|^\*$|^\/$/.test(operator)) {
    throw Error('Third parameter invalid. Valid are + | - | * | /');
  }

  return eval(num1 + operator + num2);
}

Then I've had another idea for the implementation which uses the ES6-syntax:

function calculate2(num1, num2, operator) {
  num1 = parseFloat(num1);

  if (isNaN(num1)) {
    throw Error('First parameter invalid. Must be a number.');
  }

  num2 = parseFloat(num2);

  if (isNaN(num2)) {
    throw Error('Second parameter invalid. Must be a number.');
  }

  operator = operator.trim();

  if (!/^\+$|^\-$|^\*$|^\/$/.test(operator)) {
    throw Error('Third parameter invalid. Valid are + | - | * | /');
  }

  var calculations = {
    '+' : (numb1, numb2) => numb1 + numb2,
    '-' : (numb1, numb2) => numb1 - numb2,
    '*' : (numb1, numb2) => numb1 * numb2,
    '/' : (numb1, numb2) => numb1 / numb2
  }

  return calculations[operator](num1, num2);
}

I still like the eval-version a bit better.

Can't see any harm which eval should create here because it's controlled via regular expression what is passed until that point.

What's your opinion?

Which implementation is better one?

Looking forward reading your answers.

I would go with the second one. Not because eval here is insecure -- you're validating the arguments -- but because it's overkill. eval is designed to parse and run any JavaScript code, so it is slow (compared to normal JavaScript code); engines do a lot of optimisation before running code, but they can't optimise if the code is in a dynamic string which changes at runtime. It probably won't maker much difference for you here, but if you're building a larger app, blocking JavaScript can be noticeable.

Here are the changes I would make to calculate2:

'use strict'; // Strict mode === good

function calculate2(num1, num2, operator) {
  // Number.parseFloat is new in ES6
  // (it's identical to window.parseFloat, so you can use either)
  num1 = Number.parseFloat(num1);
  num2 = Number.parseFloat(num2);
  operator = operator.trim();

  // Avoid `var`. ES6 brings `let` and `const`, which:
  //   - are block scoped
  //   - work with destructuring
  //   - prevent redeclaration in the same scope
  //   - aren't hoisted
  // Note that `const` doesn't freeze object properties
  // If you need frozen properties, use Object.freeze
  const calculations = {
    // Shorter names, it's clear from the context what they do.
    '+' : (a, b) => a + b,
    '-' : (a, b) => a - b,
    '*' : (a, b) => a * b,
    '/' : (a, b) => a / b, // You can use trailing commas in object and array literals if you want.
  }; // If you use semicolons, use them all the time.

  // Detailed error messages.
  //
  // Number.isNan is also new in ES6,
  // however it's not quite the same as window.isNan.
  // Here it won't make a difference which you use.
  //
  // `throw new Error` rather than `throw Error`
  // as you're using the Error constructor.
  if (Number.isNan(num1)) throw new Error(
    `Expected first argument to calculate2 to be a String valid for Number.parseFloat but got ${num1}.`);
  if (Number.isNan(num2)) throw new Error(
    `Expected second argument to calculate2 to be a String valid for Number.parseFloat but got ${num2}.`);

  // Don't bother maintaining a separate regex of valid operators
  // Just check the calculations object
  if (!calculations[operator]) throw new Error(
    `Expected third argument to calculate2 to be one of ${Object.keys(calculations).join(' ')} but got ${operator}.`);

  return calculations[operator](num1, num2);
}


WordPress - meta description Here's the code I'm planning to use to power meta description tags for a site in header.php in the template.  Is anything missing / incorrect / could be done better?

<?php
if (is_front_page() || is_home()) {
    $description = get_bloginfo('description');
} else {
    $description = get_post_meta(get_the_ID(), 'description', true);
}
if ($description) {
    ?>
    <meta name="description" content="<?php echo htmlspecialchars($description); ?>" />
    <?php
}
?>

In case anyone's wondering if meta descriptions have any value these days:

  Google will sometimes use the meta description of a page in search
  results snippets, if we think it gives users a more accurate
  description than would be possible purely from the on-page content.
  Accurate meta descriptions can help improve your clickthrough...

Source

EDIT:

A little explanation is I want to make sure there aren't any cases where this breaks something or doesn't print a description when it should or prints the wrong description.  I've tested it and it seems to work pretty well, but you don't know what you don't know.

What if your $description variable is empty? You are only checking if you're on the front page, or home page, and set it to blog description from wordpress, and if you're not, you're setting it to your post meta value.

What about categories? Tags? other archive pages?

The

if($description)

Will return what? False. Why? Because it's a description, not a boolean.

You should check it with

if(isset($description) && $description !== '')

If the variable is set and not empty string you'll get a true as a value that will pass the if test.

I'd do it like this:

<?php
if (is_page() || is_single()) {
    $description = get_post_meta(get_the_ID(), 'description', true);
} else {
    $description = get_bloginfo('description');
}

if (isset($description) && $description !== '') {
    ?>
    <meta name="description" content="<?php echo htmlspecialchars($description); ?>" />
    <?php
}
?>

If you're on a single post or a page, you'll get the post meta, and if you're anywhere else, you'll get blog description. And then output it only if it's set and not empty.

My advice is to see all the cases that can come up with conditional tags and set your description accordingly ;)


Fluid tag for displaying GitHub Gists with noscript tags in a Jekyll post A while back I wrote a Fluid tag for Jekyll that inserts the contents of a GitHub Gist into the content of the page in  tags for RSS readers. I've noticed a couple things I'm not happy with since posting it, but I am interested in the critique it will generate here on Stack Exchange!

(Obviously, some of the internals regarding methods required to make Liquid tags work (such as the parameters to initialize and having a render method) aren't negotiable, but you get the idea.)

require 'digest/md5'
require 'net/https'
require 'uri'

module Jekyll
  class GistTag < Liquid::Tag
    def initialize(tag_name, text, token)
      super
      @text         = text
      @cache        = true
      @cache_folder = File.expand_path "../_gist_cache", File.dirname(__FILE__)
    end

    def render(context)
      return "" unless @text =~ /([\d]*) (.*)/

      gist, file = $1.strip, $2.strip
      script_url = "https://gist.github.com/#{gist}.js?file=#{file}"

      code       = get_cached_gist(gist, file) || get_gist_from_web(gist, file)
      code       = code.gsub "<", "&lt;"
      string     = ""
      string    += "
#{code}
"
      return string
    end

    def get_gist_url_for(gist, file)
      "https://gist.github.com/raw/#{gist}/#{file}"
    end

    def cache_gist(gist, file, data)
      file = get_cache_file_for gist, file
      File.open(file, "w+") do |f|
        f.write(data)
      end
    end

    def get_cached_gist(gist, file)
      return nil if @cache == false
      file = get_cache_file_for gist, file
      return nil unless File.exist?(file)
      return File.new(file).read
    end

    def get_cache_file_for(gist, file)
      bad_chars = /[^a-zA-Z0-9\-_\.]/
      gist      = gist.gsub bad_chars, ''
      file      = file.gsub bad_chars, ''
      md5       = Digest::MD5.hexdigest "#{gist}-#{file}"
      File.join @cache_folder, "#{gist}-#{file}-#{md5}.cache"
    end

    def get_gist_from_web(gist, file)
      gist_url          = get_gist_url_for(gist, file)
      raw_uri           = URI.parse(gist_url)
      https             = Net::HTTP.new(raw_uri.host, raw_uri.port)
      https.use_ssl     = true
      https.verify_mode = OpenSSL::SSL::VERIFY_NONE
      request           = Net::HTTP::Get.new(raw_uri.request_uri)
      data              = https.request(request)
      data              = data.body
      cache_gist(gist, file, data) unless @cache == false
      data
    end
  end

  class GistTagNoCache < GistTag
    def initialize(tag_name, text, token)
      super
      @cache = false
    end
  end
end

Liquid::Template.register_tag('gist', Jekyll::GistTag)
Liquid::Template.register_tag('gistnocache', Jekyll::GistTagNoCache)

A couple of mostly minor notes and improvements:

return "" unless @text =~ /([\d]*) (.*)/
gist, file = $1.strip, $2.strip

I generally try to avoid magic variables where ever possible. Using match and the resulting MatchData object tends to be less error prone.

code       = code.gsub "<", "&lt;"

Is there a reason you only escape < characters? Unless I'm missing something, it seems preferable to use CGI.escapeHTML instead.

return string

It seems inconsistent to me that in some methods you have a return statement on the last line and in others you do not. I would always leave out the return statement on the last line as it is unnecessary.

return nil if @cache == false

It is pretty unidiomatic to use == with boolean literals - usually one would just negate the boolean instead of comparing to false. if !@cache or better unless @cache will read more natural to most people than if @cache == false.

File.new(file).read

You should never use File.new without storing its return value somewhere and then later calling close on it. As a matter of fact in most cases you should not use File.new at all, but rather File.open with a block, which closes the file automatically when the block's end is reached. The way you're doing it the file handle will remain open until the File object is garbage collected, which may be a while. This is a bad practice and can lead to subtle problems.

However in this case you don't even need File.open as there is already a method which does exactly what you need: File.read(file).

gist.gsub! /[^a-zA-Z0-9\-_\.]/, ''

You don't need to escape . as it has no special meaning inside brackets.

cache_gist(gist, file, data) unless @cache == false

This would really be a lot more readable without the explicit comparison and the double negation:

cache_gist(gist, file, data) if @cache


LINQ statements that aggregate sums from a List of objects I'm packaging data for use in a JavaScript charting library that expects a somewhat odd structure of data in which there is an array of labels and an array of dataItems where each data item has a label and an array of data that corresponds to the array of labels.  I'm creating the dataItems from a list of objects in which each one has a year, week, location, program, and hours.  I'm creating the labels by getting distinct year/week combinations, and then aggregating the data to get distinct program/location items each with an array of the sums of the hours when grouped by year/week.

I've got these two LINQ statements that I'm using to create just the data items that I believe could be one, but I can't figure out how it would be possible.  I'm summing the hours for each Program and Location for each Year and Week, and then I need an array of the sums for each Location and Program.

var dataRows = from h in hours
               orderby h.Year, h.WeekOfYear, h.LocationName, h.ProgramName
               group h by new { h.Year, h.WeekOfYear, h.LocationName, h.ProgramName } into programLocationGroups
               select new { Location = programLocationGroups.Key.LocationName, Program = programLocationGroups.Key.ProgramName, Hours = programLocationGroups.Sum(p => p.Hours) };

chartData.DataRows = (from d in dataRows
                      group d by new { d.Program, d.Location } into programLocationSums
                      select new FindAllAnalyticsResponse.DataRow
                      {
                          Location = programLocationSums.Key.Location,
                          Program = programLocationSums.Key.Program,
                          Data = programLocationSums.Select(p => p.Hours).ToList()
                      }).ToList();

I'd love to see a more concise solution if it exists.  There's also room for improvement due to the fact that missing data will result in mismatched arrays (not possible in the current environment, but something I'm aware of).

To combine the two queries into one, you can use select … into:

chartData.DataRows = (from h in hours
                      orderby h.Year, h.WeekOfYear, h.LocationName, h.ProgramName
                      group h by new { h.Year, h.WeekOfYear, h.LocationName, h.ProgramName }
                      into programLocationGroups
                      select new
                      {
                          Location = programLocationGroups.Key.LocationName,
                          Program = programLocationGroups.Key.ProgramName,
                          Hours = programLocationGroups.Sum(p => p.Hours)
                      }
                      into d
                      group d by new { d.Program, d.Location } into programLocationSums
                      select new FindAllAnalyticsResponse.DataRow
                      {
                          Location = programLocationSums.Key.Location,
                          Program = programLocationSums.Key.Program,
                          Data = programLocationSums.Select(p => p.Hours).ToList()
                      }).ToList();

Though I'm not sure this is actually an improvement.

I think an equivalent and simpler query would be to first group only by location and program and then by week in a nested query:

chartData.DataRows = (from h in hours
                      orderby h.LocationName, h.ProgramName
                      group h by new { h.LocationName, h.ProgramName } into programLocationGroup
                      select new FindAllAnalyticsResponse.DataRow
                      {
                          Location = programLocationGroup.Key.LocationName,
                          Program = programLocationGroup.Key.ProgramName,
                          Data = (from pl in programLocationGroup
                                  orderby pl.Year, pl.WeekOfYear
                                  group pl by new { pl.Year, pl.WeekOfYear } into weekGroup
                                  select weekGroup.Sum(w => w.Hours)).ToList()
                      }).ToList();

Note that the above code may need some better in the variables I introduced.


Sorting colors by the name of the color I was in need to sort Colors, so the first thing that I tried was this:

Arrays.sort(new javafx.scene.paint.Color[] {Color.ALICEBLUE},Color.AZURE,Color.BLUEVIOLET,Color.AQUA,...);

But unfortunately Arrays.Sort is not compatible with javafx.scene.paint.Color, so I ended up with this:

public static void main(String[] args) {
    final String[] colors = {"AQUA", "LAWNGREEN", "LIGHTBLUE", "CYAN", "CHOCOLATE", "GREEN",
        "RED", "YELLOW", "GAINSBORO", "YELLOWGREEN"};

    final Function<String, ? extends Color> valueMapper = (final String name) -> {
        try {
            return (Color) Color.class.getField(name).get(null);
        } catch (final IllegalAccessException | IllegalArgumentException | NoSuchFieldException | SecurityException e) {
            throw new RuntimeException(e);
        }
    };
    final Map<String, Color> map = Arrays.stream(colors).collect(
            Collectors.toMap(Function.identity(), valueMapper));

    map.entrySet().stream().sorted(Comparator.comparing(Entry::getKey)).forEach((Entry<String, Color> entry) -> {
        System.out.println(entry.getKey() + " = " + entry.getValue());
    });

}

Now let me know if this code can be improved and optimized.

I like your approach with mapping function, but sorting alphabetically the collection of javafx.scene.paint.Color objects can be done a little bit easier:

    final String[] colors = {"AQUA", "LAWNGREEN", "LIGHTBLUE", "CYAN", "CHOCOLATE", "GREEN",
            "RED", "YELLOW", "GAINSBORO", "YELLOWGREEN"};

    Stream.of(colors).sorted().map(color -> Color.web(color)).forEach(System.out::println);

as you can see the algorithm quite simple:

Sort the string array
Map string to Color object using API method


capturing counts of open items between dates DB version is: Oracle 12c. Restricted to the use of inline PL/SQL.

The end goal here is to get a count of open items by day, inclusive of created date, exclusive of the end date.

The data structure is approximately:

| id | open_date  | close_date |
|----|------------|------------|
| a  | 01/01/2020 | 01/04/2020 |
| b  | 01/02/2020 | 01/05/2020 |

The end result would be something like:

| date       | open_item |
|------------|-----------|
| 01/01/2020 | 1         |
| 01/02/2020 | 2         |
| 01/03/2020 | 2         |
| 01/04/2020 | 1         |
| 01/05/2020 | 0         |

the start date in all this is user input, but that's not terribly relevant.

What I had done is generate a table of dates using that user input date in a cte like so:

with caledar_dates as (
select user_input_date + rownum-1 dates

from dual connect by rownum < sysdate-user_input_date+1
)

Then joined the items table to the calendar_dates table in another CTE like so:

item_list as (
    select
    a.dates,
    b.id

    from calendar_dates a

    left join ticket_table b on a.dates>=b.created_date-1 and a.dates<b.closed_date

)

Finally, I then simply

select count(b.id), dates from item_list group by dates

There may be a few syntax errors above since I am truncating the actual code. This query works; however, it seems like an inefficient, wordy, ugly approach to the problem and if the user inputs a date a few years back, the query takes a few minutes to run.

Looking for an alternative and hopefully more efficient approach.

Not sure if this went on SO or here so happy to post over there if that seems the better venue.

Mahalo's in advance.

In looking at your query, I think there may be a few things you can do to improve your performance.  The first is that you can aggregate your ticket_table ahead of time based on the request interval. Second, unless it is needed for the logic that you redacted, I would avoid using the CTE once you start doing the real logic. You may be depriving the optomizer of information it could use.  You might try something like the following query:

WITH query_interval AS
(
  SELECT TO_DATE('09/01/2012', 'MM/DD/YYYY') AS START_DATE, TRUNC(SYSDATE) AS END_DATE
  FROM DUAL
),
interval_dates AS
(
  SELECT qi.start_date + ROWNUM - 1 AS I_DATE
  FROM query_interval qi
  CONNECT BY ROWNUM < qi.end_date - qi.start_date
)
SELECT i.i_date, SUM(sub.cycle_count)
FROM interval_dates i
LEFT JOIN (SELECT GREATEST(TRUNC(tt.start_date), qi.start_date) AS OPEN_DATE,
                 LEAST(TRUNC(tt.end_date)-1, qi.end_date) AS CLOSE_DATE,
                 COUNT(*) AS CYCLE_COUNT
          FROM ticket_table tt
          CROSS JOIN query_interval qi
          WHERE GREATEST(TRUNC(tt.start_date), qi.start_date) <= LEAST(TRUNC(tt.end_date)-1, qi.end_date)
          GROUP BY GREATEST(TRUNC(tt.start_date), qi.start_date), LEAST(TRUNC(tt.end_date)-1, qi.end_date)) sub ON I_DATE BETWEEN sub.open_date AND sub.close_date
GROUP BY i.i_date
ORDER BY i.i_date;

Most of this is similar to what you already had, but there are some differences.  First you notice that I added a CTE with just the interval.  This allows me to perform the aggregation that I mentioned earlier.  The aggregation works like this:

First, we limit our search to tickets which were open at some point during our query interval.  That I'm using the GREATEST <= LEAST to determine an overlap between the ticket and the query interval.
We clip tickets that hang over the ends of our search interval.  You really don't care that they are open before your search, just that they are open during your search.
We aggregate all cycles which now have the same start and end dates.

After that, the logic is pretty similar to what you already had.  A left join between the full list of dates and our modified table.  Then instead of using COUNT, we use SUM.


Runge-Kutta 4th order using Python numexpr.evaluate() I am implementing an ODE solver, where the user provides rates and coefficients as a string. ODE solver has to work with vectors.  The best implementation I got so far is the following:

# k_1 = dt*dcdt(C0, dt);
# k_2 = dt*dcdt(C0+0.5*k_1, dt);
# k_3 = dt*dcdt(C0+0.5*k_2, dt);
# k_4 = dt*dcdt(C0+k_3, dt);
# C_new = C0 + (k_1+2*k_2+2*k_3+k_4)/6;

import numpy as np
import numexpr as ne

dt = 0.1

coef = {'k': 1}
rates = {'R1': 'k*y1*y2'}
dcdt = {'y1': '-4 * R1', 'y2': '-R1'}
C0 = {'y2': np.random.rand(400), 'y1': np.random.rand(400)}

def k_loop(conc):
    rates_num = {}

    for k in rates:
        rates_num[k] = ne.evaluate(rates[k], {**coef, **conc})

    dcdt_num = {}

    for k in dcdt:
        dcdt_num[k] = ne.evaluate(dcdt[k], rates_num)

    Kn = {}

    for k in C0:
        Kn[k] = dt * dcdt_num[k]
    return Kn

def sum_k(A, B, b):
    C = {}
    for k in A:
        C[k] = A[k] + b * B[k] * dt
    return C

def result(C_0, k_1, k_2, k_3, k_4):
    C_new = {}
    for k in C_0:
        C_new[k] = C_0[k] + (k_1[k] + 2 * k_2[k] + 2 * k_3[k] + k_4[k]) / 6
    return C_new

k1 = k_loop(C0)
k2 = k_loop(sum_k(C0, k1, 0.5))
k3 = k_loop(sum_k(C0, k2, 0.5))
k4 = k_loop(sum_k(C0, k3, 1))

C_new = result(C0, k1, k2, k3, k4)

This ODE solver I am planning to use inside of the Object-oriented PDE solver. The PDE solver will provide C0 vectors. Therefore, the performance is crucial. I used numexpr.evaluate() for simplified user input of the rates. Can it be implemented in vectorized form? If you know the better implementation of the solver with such simplified input, please, let me know.

Metrics:

eval():

1000 loops, best of 3: 372 µs per loop

ne.evaluate():

1000 loops, best of 3: 377 µs per loop

@Graipher example:

1000 loops, best of 3: 452 µs per loop

I would make all objects numpy arrays and get the user to use index notation. They would have to write equations of the form R = np.array(['c[0]*y[0]*y[1]']) meaning that this will later evaluate just like your R = {'R1': 'c*y1*y2'}. Almost all functions can then take advantage of the fact that numpy does almost everything vectorized.

#!/usr/bin/env python3
# k_1 = dt*dcdt(C0, dt);
# k_2 = dt*dcdt(C0+0.5*k_1, dt);
# k_3 = dt*dcdt(C0+0.5*k_2, dt);
# k_4 = dt*dcdt(C0+k_3, dt);
# C_new = C0 + (k_1+2*k_2+2*k_3+k_4)/6;

import numpy as np

dt = 0.1

c = np.array([1])
y = np.array([[0, 0.1, 0.2, 0.3], [0, 0.1, 0.2, 0.3]])
R = np.array(['c[0]*y[0]*y[1]'])
dcdt = np.array(['-4 * R[0]', '-R[0]'])

def k_loop(y):
    rates_num = np.array([eval(k) for k in R])
    dcdt_num = np.array([eval(k, {'R': rates_num}) for k in dcdt])
    return dt * dcdt_num

def sum_k(A, B, b):
    return A + b * B * dt

def result(y_0, k_1, k_2, k_3, k_4):
    return y_0 + (k_1 + 2 * k_2 + 2 * k_3 + k_4) / 6

k1 = k_loop(y)
k2 = k_loop(sum_k(y, k1, 0.5))
k3 = k_loop(sum_k(y, k2, 0.5))
k4 = k_loop(sum_k(y, k3, 1))

y_new = result(y, k1, k2, k3, k4)
print(y_new)

Note that this is slightly slower for the small example given (0.09s vs 0.08s). Also the step rates_num = np.array([eval(k) for k in R]) looses some precision (up to 1E-4).

To see whether or not this performs better, a more realistic example would be needed.

The eval call can be back-substituted with ne.evaluate, I just didn't want to install it.

To calculate timings, I ran the result function 100000 times, like so:

for _ in range(100000):
    y = result(y, k1, k2, k3, k4)

When doing this with your original code, this takes 1.36s, while my code takes 0.99s.

However, when including updating the ks, this advantage goes away again:

for _ in range(10000):
    k1 = k_loop(y)
    k2 = k_loop(sum_k(y, k1, 0.5))
    k3 = k_loop(sum_k(y, k2, 0.5))
    k4 = k_loop(sum_k(y, k3, 1))
    y = result(y, k1, k2, k3, k4)

Note that this is ten times less often than before. Here your code takes 2.7s and my code takes 3.7s on my machine.


New Year Chaos JavaScript, needs to be sped up Similar to this question but this is for Python

Original problem with description on hacker rank

I am currently trying to iterate through a large number of arrays and count how many times numbers have been swapped.

A sorted array looks like this: [1,2,3,4,5] and a number can be swapped only towards the front (counting down to 0) twice.

If a number is more than 2 out of order the array is deemed 'Too chaotic' and the process should stop.

Instead of bubble sorting I am simply going through and counting the actual swaps. As a sorted array is not actually required, my code works except for a couple of the tests where it times out due to large arrays.

Any ideas on how to speed this process up?

function minimumBribes(q) {
console.log(sort(q));

    function sort(items) {
     let bribes = 0;

     for (let i = 0; i < items.length; i++) {
         if (items[i] - (i + 1) > 2) return "Too chaotic";
         for (let j = 0; j < i; j++) {
             if (items[j] > items[i]) bribes++;
         }
     }
     return bribes;
   }
}

Reduce the inner loops iteration count.
The problem is the inner loop is looping over too many items. The result is that you spend too much time processing data you know is irrelevant.
As the function should exit if it detects a position has made over 2 bribes, you need only have the inner loop check positions down 2 from the item you are checking, and not from the start of the line.
Quicker solution
It only requires a slight modification of your code, but as you have complicated the situation by calling an inner function sort the example has just removed the inner function.
The line for (j = pos-2; j < i; j++) { is where the improvement is with pos being item[i] in your function.
function minBribe(queue) {
    var bribes = 0, i, j;
    for (i = 0; i < queue.length; i++) {
        const pos = queue[i], at = i + 1;
        if (pos - at > 2) { return "Too chaotic" }
        for (j = Math.max(0, pos - 2); j < i; j++) {
            if (queue[j] > pos) { bribes++ }
        }
    }
    return bribes;
}

This brings the solution down from \$O(n^2)\$ to near \$O(n)\$ however the number of bribes is a factor so its closer to \$O(n + (m^{0.5}/2))\$ where \$m\$ is the number of bribes. (this is only an approximation)


Hacker Rank: Extracting digits from a given number and check for divisibility The problem in question is a coding challenge from Hackerrank:

  Problem Statement

  You are given an integer N. Find the digits in this number that
  exactly divide N (division that leaves 0 as remainder) and display
  their count. For N=24, there are 2 digits (2 & 4). Both of these
  digits exactly divide 24. So our answer is 2.

  Note

  If the same number is repeated twice at different positions, it should
  be counted twice, e.g., For N=122, 2 divides 122 exactly and occurs at
  ones' and tens' position. So for this case, our answer is 3. Division
  by 0 is undefined. Input Format

  The first line contains T (the number of test cases), followed by T
  lines (each containing an integer N).

  Constraints
  1≤T≤15
   0 < N <10 ^ 10

  Output Format

  For each test case, display the count of digits in N that exactly
  divide N in a separate line.

Here's my code:

package algorithms.Warmup;

import java.util.ArrayList;
import java.util.Scanner;

/**
 * Created by user1 on 4/2/15.
 */
public class FindDigitsTest {

    static Scanner sc = new Scanner(System.in);

    private static Long[] findDigits(long num) {

        ArrayList<Long> digits = new ArrayList<>();

        //extract all digits from input number and store in arraylist
        while (num > 0) {

            long quotient = num / 10;
            long remainder = num - (quotient * 10);
            num = quotient;
            digits.add(remainder);

        }

        Long[] result = new Long[digits.size()];
        result = digits.toArray(result) ;
        return result;

    }

    private static int findCount(Long[] list, long num) {

        int count = 0;
        for(int i = 0; i < list.length; i++) {

            //ignore division by zero- DON'T FORGET!!!
            if(list[i] == 0) {
                continue;
            }
            if (num % list[i] == 0){
                count++;
            }
        }

        return count;

    }
    public static void main(String[] args) {

        ArrayList<Integer> al = new ArrayList<>();

        //read in number of test cases
        System.out.println("Enter number of test cases: ");
        int T = sc.nextInt();

        //loop through number of test cases
        for(int i = 0; i < T; i++) {

            System.out.println("Enter the number: ");
            long N = sc.nextLong();

            //extract digits of number and store in array
            Long[] nDigits = findDigits(N);

            //calculate num. of digits which exactly divide N
            int answer = findCount(nDigits, N);

            //store answer in final arraylist
            al.add(answer);

        }

        //loop through final arraylist to print answers
        for(int i = 0; i < al.size(); i++) {
            System.out.println(al.get(i));
        }

    }
}

Even though the submission is correct, and the output is as desired, I feel there are many ways this can be improved, I just don't know how. I see that I have too many ArrayLists and Arrays here and there throughout the code, maybe that can be replaced to begin with.

I'm not completely proficient in Java yet, so my knowledge of Java APIs and Collections is still somewhat limited. That might be one reason for the inefficient code.

I see that I have too many ArrayLists and Arrays here and there throughout the code, maybe that can be replaced to begin with.

Yes, that would probably be a good idea. In findDigits you internally use an ArrayList, I'm guessing because it's a lot easier to use than an array (which is true, a List is the correct collection type here). But then you transform it to an array, which isn't really needed.

Returning lists instead of arrays is perfectly acceptable in Java, and is in fact used in most cases (an exception might be made for fixed length arrays in performance critical code, eg coordinates).

So just change your signature from Long[] findDigits(long num) to List<Long> findDigits(long num), and then use a List in findCount as well.

Misc

declare variables in as small a scope as possible. sc isn't really needed anywhere except in main, so declare it there.
don't use short variables names. sc would be clearer as scanner and al could be results.
findDigits could be getDigits, after all you are not really searching for the digits or computing them (they are right there), just getting them.
findCount could be countDivisors, and then num could be divisor.
the question says that you are given an integer, but you use long instead.
your while loop in findDigits could be simplified to digits.add(num % 10);  num = num / 10;.


How to remove duplicated code? AOP and Generics are Ok I have figures - TriangleItem and CircleItem:

public abstract class BaseItem
{
  protected int _id;
  protected string _description;
}

public sealed class TriangleItem : BaseItem
{
  public int TriangleId { get { return _id; } set { _id = value; } }
  public string TriangleDescription { get { return _description; } set { _description = value; } }
}

public sealed class CircleItem : BaseItem
{
  public int CircleId { get { return _id; } set { _id = value; } }
  public string CircleDescription { get { return _description; } set { _description = value; } }
}

And I have figure collections TriangleBox and CircleBox:

public abstract class BaseBox
{
  public string ItemsXml { get; set; }
  public string Descriptions { get; set; }
  public static abstract BaseBox Boxing( BaseItem[] items );
  public abstract void Unboxing();
}

public sealed class TriangleBox : BaseBox
{
  public static override BaseBox Boxing( BaseItem[] items )
  {
    string[] ids = items.ToList().Cast<TriangleItem>().Select( i => i.TriangleId.ToString() ).ToArray();
    string[] descriptions = items.ToList().Cast<TriangleItem>().Select( i => i.TriangleDescription ).ToArray();

    return new TriangleBox()
    {
      ItemsXml = Util.ToXml( ids ),
      Descriptions = descriptions.Aggregate( ( d, next ) => next + ", " + d )
    };
  }

  public TriangleItem[] Unboxing()
  {
    return Util.FromXml( ItemsXml ).Select( i => new TriangleItem { TriangleId = int.Parse( i ), TriangleDescription = i } ).ToArray();
  }
}

public sealed class CircleBox : BaseBox
{
  public static override BaseBox Boxing( BaseItem[] items )
  {
    string[] ids = items.ToList().Cast<CircleItem>().Select( i => i.CircleId.ToString() ).ToArray();
    string[] descriptions = items.ToList().Cast<CircleItem>().Select( i => i.CircleDescription ).ToArray();

    return new CircleBox()
    {
      ItemsXml = Util.ToXml( ids ),
      Descriptions = descriptions.Aggregate( ( d, next ) => next + ", " + d )
    };
  }

  public CircleItem[] Unboxing()
  {
    return Util.FromXml( ItemsXml ).Select( i => new CircleItem { CircleId = int.Parse( i ), CircleDescription = i } ).ToArray();
  }
}

As you can see, I have almost the same method implementations - Boxing and Unboxing.
I use these methods as follows:

  public BaseBox Do( BaseItem[] items )
  {
    BaseBox result = null;

    if( items is CircleItem[] )
      result = CircleBox.Boxing( items );
    else if( items is TriangleItem[] )
      result = TriangleBox.Boxing( items );

    return result;
  }

  public TriangleItem[] GetTriangles()
  {
    TriangleBox tb = DB.GetTriangleBox(); // From database
    return tb.Unboxing();
  }

  public CircleItem[] GetCircles()
  {
    CircleBox cb = DB.GetCircleBox(); // From database
    return cb.Unboxing();
  }

Can I changle something to have only one Boxing and Unboxing implementations?
What would you change in this code? What do you dislike (except catch null method's parameters)? AOP and Generics are OK if need.

In Do-method I have to know items's type. And I have to return empty TriangleBox or CircleBox if items.Length is 0. How I have to write my Do-method with this case?

I assume you really need to distinguish Circle and Triangle.
Any way you can use auto-properties of base class.

public abstract class BaseItem
{
    public int Id { get; set; }
    public string Description { get; set; }
}

public sealed class TriangleItem : BaseItem
{
    // Another TriangleItem class members
}

public sealed class CircleItem : BaseItem
{
    // Another CircleItem class members
}

Boxing mehtods differ in type of object they return, which is subtype of BaseBox though.
That means you can merge them and delegate particular boxes creation to subclasses.

public abstract class BaseBox
{
    public string ItemsXml { get; set; }
    public string Descriptions { get; set; }

    public BaseBox Boxing(BaseItem[] items)
    {
        var ids = items.ToList().Select(i => i.Id.ToString()).ToArray();
        var descriptions = items.ToList().Select(i => i.Description.ToString()).ToArray();
        var box = CreateBox(Util.ToXml(ids), descriptions.Aggregate((d, next) => next + ", " + d));
        return box;
    }

    public BaseItem[] Unboxing()
    {
        return Util.FromXml(ItemsXml).Select(i => CreateItem(int.Parse(i), i)).ToArray();
    }

    protected abstract BaseBox CreateBox(string itemsXml, string descriptions);

    protected abstract BaseItem CreateItem(int id, string description);
}

The same can be done for unboxing and BaseItem.

public sealed class TriangleBox : BaseBox
{
    protected override BaseBox CreateBox(string itemsXml, string descriptions)
    {
        return new TriangleBox { ItemsXml = itemsXml, Descriptions = descriptions };
    }

    protected override BaseItem CreateItem(int id, string description)
    {
        return new TriangleItem { Id = id, Description = description };
    }
}

public sealed class CircleBox : BaseBox
{
    protected override BaseBox CreateBox(string itemsXml, string descriptions)
    {
        return new CircleBox { ItemsXml = itemsXml, Descriptions = descriptions };
    }

    protected override BaseItem CreateItem(int id, string description)
    {
        return new CircleItem { Id = id, Description = description };
    }
}

Update:
Boxing method should be void. And probably renamed to 'DoBoxing'.

public abstract class BaseBox
{
    ...

    public void Boxing(BaseItem[] items)
    {
        var ids = items.ToList().Select(i => i.Id.ToString()).ToArray();
        var descriptions = items.ToList().Select(i => i.Description.ToString()).ToArray();
        this.ItemsXml = Util.ToXml(ids);
        this.Descriptions = descriptions.Aggregate((d, next) => next + ", " + d);
    }

    ...
}

And called as:

    public BaseBox Do(BaseItem[] items)
    {
        ...

        if (result != null)
        {
            result.Boxing(items);
        }

        return result;
    }


Minimizing duplicate routes index.js I am trying to build a REST API with express router, which contains of nested sub routes. I have mounted these sub routes in my index.js file.

I have defined it as follows:

// Mounted routes
app.use('/api/v1/Project', new ProjectRouter().routes);
app.use('/api/v1/Project/:projectId/Context', new ContextRouter().routes);
app.use('/api/v1/Project/:projectId/Context/:contextId/Question', new QuestionRouter().routes);
app.use('/api/v1/Project/:projectId/Context/:contextId/Question/:questionId/Answer', new AnswerRouter().routes);

I want to arrange my routes revolved around the functionality and being more complaint towards REST standards.

In the case the route prefix /api/v1/Project/ is being repeated over and over again.

Is there some best practice to minimize the redundant routes by prefixing?

This is where the Express Router class comes in.  You could define a router for ‘api/v1/Project’, mount that router to you main app, and then add the individual routes to the router.


Printing hex dump of a byte array The goal is readabilty. Performance is not much of a concern, since it's not going to be used with large amount of data.
Except for degenerate case when the length of the incoming byte array is zero, the dump does not end with a new line character. This is regardless whether ASCII part of the dump is shown or not. The code is also careful not to put any trailing spaces in dump lines, since the dump is intended to be copy-pasted and used as part of other texts.

Code:

class Hex
{
    private readonly byte[] _bytes;
    private readonly int _bytesPerLine;
    private readonly bool _showHeader;
    private readonly bool _showOffset;
    private readonly bool _showAscii;

    private readonly int _length;

    private int _index;
    private readonly StringBuilder _sb = new StringBuilder();

    private Hex(byte[] bytes, int bytesPerLine, bool showHeader, bool showOffset, bool showAscii)
    {
        _bytes = bytes;
        _bytesPerLine = bytesPerLine;
        _showHeader = showHeader;
        _showOffset = showOffset;
        _showAscii = showAscii;
        _length = bytes.Length;
    }

    public static string Dump(byte[] bytes, int bytesPerLine = 16, bool showHeader = true, bool showOffset = true, bool showAscii = true)
    {
        if (bytes == null)
        {
            return "<null>";
        }
        return (new Hex(bytes, bytesPerLine, showHeader, showOffset, showAscii)).Dump();
    }

    private string Dump()
    {
        if (_showHeader)
        {
            WriteHeader();
        }
        WriteBody();
        return _sb.ToString();
    }

    private void WriteHeader()
    {
        if (_showOffset)
        {
            _sb.Append("Offset(h)  ");
        }
        for (int i = 0; i < _bytesPerLine; i++)
        {
            _sb.Append($"{i & 0xFF:X2}");
            if (i + 1 < _bytesPerLine)
            {
                _sb.Append(" ");
            }
        }
        _sb.AppendLine();
    }

    private void WriteBody()
    {
        while (_index < _length)
        {
            if (_index % _bytesPerLine == 0)
            {
                if (_index > 0)
                {
                    if (_showAscii)
                    {
                        WriteAscii();
                    }
                    _sb.AppendLine();
                }

                if (_showOffset)
                {
                    WriteOffset();
                }
            }

            WriteByte();
            if (_index % _bytesPerLine != 0 && _index < _length)
            {
                _sb.Append(" ");
            }
        }

        if (_showAscii)
        {
            WriteAscii();
        }
    }

    private void WriteOffset()
    {
        _sb.Append($"{_index:X8}   ");
    }

    private void WriteByte()
    {
        _sb.Append($"{_bytes[_index]:X2}");
        _index++;
    }

    private void WriteAscii()
    {
        int backtrack = ((_index-1)/_bytesPerLine)*_bytesPerLine;
        int length = _index - backtrack;

        // This is to fill up last string of the dump if it's shorter than _bytesPerLine
        _sb.Append(new string(' ', (_bytesPerLine - length) * 3));

        _sb.Append("   ");
        for (int i = 0; i < length; i++)
        {
            _sb.Append(Translate(_bytes[backtrack + i]));
        }
    }

    private string Translate(byte b)
    {
        return b < 32 ? "." : Encoding.GetEncoding(1252).GetString(new[] {b});
    }
}

Usage:

Console.OutputEncoding = Encoding.GetEncoding(1252);
byte[] example = Enumerable.Range(0, 256).Select(x=>(byte)x).ToArray();
Console.WriteLine(Hex.Dump(example));

Sample output:

Thing I like less is that you're forcing caller to deal with a static method. I appreciate that kind of helpers but they should be an alternative (maybe with default settings) instead of the only way to go. Why?

If you need more parameters (or to reorder them) then you need to change all calling points.
If you add complex parameters (such as a formatter) then calling point will quickly become a mess.
They're viral. If you separate UI from underlying logic and you need to delegate some decisions to different places then you can't pass around an Hex class (where each component/control may fill its own properties) because you have a single method call.
If a static method becomes complex enough then it will also be harder to test.

I'd also change name from Hex to something that describe what your class is doing. HexStringFormatter? HexStringConverter? I'd give two alternatives:

var formatter = new HexStringFormatter();
Console.WriteLine(formatter.ConvertToString(example));

Note that in this simple case it may even be (an helper method here just save few characters, what's for?):

Console.WriteLine(new HexStringFormatter().ConvertToString(example));

A more complex case may be like this:

var formatter = new HexStringFormatter();
formatter.BytesPerLine = 32;
formatter.Content = HexStringFormatterOutput.Ascii
    | HexStringFormatterOutput.Header;

These settings may come from UI, now you can return formatter object to another method for example in charge to read data without knowing where settings come from...

Note that I also introduced an enum instead of multiple boolean properties but it's not mandatory and not even always suggested, it depends case by case (I like the enum because I can save it in one shot without any effort in my configuration files).

Also you may want to make Hex sealed, it's not intended (so far) to be extended.

You may accept IEnumerable<byte> instead of byte[]. Performance impact in WriteBody() with foreach is negligible, and you do not need ToArray() in your example. Not such big gain but it will let you read huge files that does not fit in memory (because you can read them block by block without need to convert to byte[] all the content).

In this regard you may want to change the output of your class. Now you need to build a huge output string, if content will be written to disk then it's a waste of resources. Write output to a TextWriter. Caller will have responsibility to give you a StringWriter if he wants to output to a string. In this case it may be nice to introduce a static helper method for the most common use case. More complex use case may be like this:

using (var output = new StringWriter())
{
    var formatter = new HexStringFormatter();
    formatter.Output = output;
    formatter.BytesPerLine = 32;

    formatter.ConvertToString(example);
    Console.WriteLine(output.ToString());
}

HOWEVER note that HexStringFormatter accepts a TextWriter and Console.Out is a TextWriter! Code will then be simplified to:

var formatter = new HexStringFormatter();
formatter.Output = Console.Out;
formatter.BytesPerLine = 32;

formatter.ConvertToString(example);

What if you need to write it to a text file?

using (var stream = new StreamWriter("path_to_file"))
{
    var formatter = new HexStringFormatter();
    formatter.Output = stream;
    formatter.BytesPerLine = 32;

    formatter.ConvertToString(example);
}

Of course you may want to introduce few helper methods for this (now they can make sense but keep them as simple as possible), something like this:

public static string DumpToString(IEnumerable<byte> data)
{
    using (var output = new StringWriter())
    {
        var formatter = new HexStringFormatter();
        formatter.Output = output;

        formatter.ConvertToString(data);
        return output.ToString();
    }
}

public static void DumpToFile(string path, IEnumerable<byte> data)
{
    using (var output = new StreamWriter(path))
    {
        var formatter = new HexStringFormatter();
        formatter.Output = output;

        formatter.ConvertToString(data);
    }
}

public static void DumpToConsole(IEnumerable<byte> data)
{
    var formatter = new HexStringFormatter();
    formatter.Output = Console.Out;

    formatter.ConvertToString(data);
}

Now your first example will be again like this:

HexStringFormatter.DumpToConsole(
    Enumerable.Range(0, 256).Select(x => (byte)x));

Few other minor things. Your Translate() function should be simplified and made static). Also do not need to get a new encoding for each call, use Encoding.ASCII. b < 32 is clear for most of us but I'd make it explicit. For now let's keep GetString() but see later...

private static string Translate(byte b)
{
    if (IsPrintableCharacter(b))
        return Encoding.ASCII.GetString(new[] { b });

    return "."
}

private static bool IsPrintableCharacter(byte b) => b > 32;

Now let's think...you're working with ASCII and byte is in range [0...255] then you may directly cast it to char (after leave to representation issues you mentioned in your Console codepage settings), you avoid to create so many strings and you do not even need any encoding:

private static char Translate(byte b)
    => IsPrintableCharacter(b) ? (char)b : '.';

Code like i & 0xFF in WriteHeader() is little bit misleading, IMO. If _bytesPerLine contains an invalid value then it should throw ArgumentOutOfRangeException when you set it, it shouldn't be silently ignored. Your code will then be simplified everywhere.

About your main loop, you do not need to save _length field because you already have the input buffer. I'd also try to give names to all those ifs (=introduce bool locals or separate functions) anyway after refactoring to use IEnumerable<byte> it should look much simpler.


Good practice using SCSS I saw a sassy CSS for a rails app like this:

$std-margin: 30px;

@mixin solid-bg($color: #EEEEEE) {
  background: none repeat scroll 0 0 $color;
}

@mixin solid-border($direction, $width: 1px, $color: black) {
  @if $direction == "" {
    border: {
      color: $color ;
      width: $width;
      style: solid;
    }
  } @else {
    border-#{$direction}: {
      color: $color ;
      width: $width;
      style: solid;
    }
  }
}

html {
  font: 90%/1.3 arial,sans-serif;
  body {
    margin: $std-margin;
    .top-nav {
      text-align: center;
      width: 100%;
      .login {
        float: right;
        padding-right: 63px;
        clear: both;
      }
      a {
        text-decoration: none;
      }
    }
    .content {
      @include solid-border("");
      clear: both;
      padding: 15px;
    }
    h2 {
      margin: $std-margin;
      margin-left: 0px;
    }
    h3, .detail {
      margin: $std-margin;
    }
    #header {
      ul {
        list-style: none outside none;
        padding: 0;
        li {
          float: left;
          a {
            @include solid-bg();
            color: #0000CC;
            display: block;
            padding: 0.24em 1em;
            text-decoration: none;
          }
        }
      }
      #selected a {
        @include solid-bg(white);
        color: black;
        font-weight: bold;
        position: relative;
        top: 1px;
        @include solid-border("");
        @include solid-border('bottom', 0px);
      }
    }
    label {
      float: left;
      margin-right: 1em;
      text-align: right;
      width: 10em;
    }
    input {
      @include solid-bg(#FAFAFA);
      -moz-border-radius: 0.4em 0.4em 0.4em 0.4em;
      @include solid-border("", 1px, #DDDDDD);
      font: bold 0.95em arial,sans-serif;
      padding: 0.15em;
      width: 10em;
    }
    .field {
      padding: 5px;
      clear: both;
      input {
        width:12em;
        @include solid-border("", 1px, #006);
        @include solid-bg(#ffc);
        &:hover {
          @include solid-border("");
          @include solid-bg(#ff6);
        }
      }
      .fieldWithErrors {
        float: left;
        padding: 3px;
        input {
        @extend .field.input;
        @include solid-border("", 2px, red);
        }
      }
      .create {
        width:5em;
      }
      .submit {
        @include solid-bg(#D0DAFD);
        border: 0 none;
        cursor: pointer !important;
        display: block;
        height: 26px;
        margin: 1em;
        overflow: hidden;
        width: 69px;
      }
    }

    table {
      border-collapse: collapse;
      margin: 20px;
      text-align: left;
      width: 480px;
      font {
       family: "Lucida Sans Unicode","Lucida Grande",Sans-Serif;
       size: 12px;
      }
      th {
        @include solid-bg(#B9C9FE);
        @include solid-border('bottom', 1px, #FFFFFF);
        @include solid-border('top', 4px, #AABCFE);
        color: #003399;
        font-size: 13px;
        font-weight: normal;
        padding: 8px;
      }
      tr {
        @include solid-bg(#E8EDFF);
        &:hover td {
          @include solid-bg(#D0DAFD);
          color: #333399;
        }
        td.title {
         color: #333399;
        }
      }
      td {
        @include solid-border('bottom', 1px, #FFFFFF);
        @include solid-border('top', 1px, transparent);
        color: #666699;
        padding: 8px;
      }
    }
    .activities {
      padding: 10px;
      a{
        margin: 10px;
      }
    }
    .flash {
      font-family:'Trebuchet MS';
      color:#FF0000;
      font-size:14px;
    }
    .calendar_date_select table {
      width: 78px;
      td {
        padding: 5px;
      }
    }
    .main-content {
      clear: both;
      float: left;
      width: 100%;
    }
    .clear {
      clear: both;
    }
    #errorExplanation {
      @include solid-bg();
      margin: 10px;
      padding: 5px 0 5px 20px;
    }
  }
}

Using best practice how this can be optimized?

One optimization you should do in any case is not to have all rules inside html { ... } and body { ... }, because the resulting CSS rules all require the renderer to unnecessarily check every single element if it is actually a decedent of html and body - which they undoubtedly are.


Production-ready comparison between byte-array and ByteBuffer I want to compare the performance difference between using byte[] and ByteBuffer in Java, which approach I should use in my production code. I wrote the following Java sandbox code:

public native int MD_GetHsmInfo(int hsmIndex, int infoType, byte[] value);
public native int MD_TestByteBuffer(int hsmIndex, int infoType, ByteBuffer buf);

//inside main()
byte[] arr = new byte[64];
h.MD_GetHsmInfo(2, 0, arr);
System.out.println(">" + new String(arr) + "<");

ByteBuffer buf = ByteBuffer.allocateDirect(64);
h.MD_TestByteBuffer(2, 1, buf);
System.out.println(">" + StandardCharsets.UTF_8.decode(buf).toString() + "<");

And then I have my C code - this is the one I need help with:

JNIEXPORT jint JNICALL Java_com_sprint_jni_JNISandbox_MD_1GetHsmInfo
    (JNIEnv *env, jobject obj, jint hsmIndex, jint infoType, jbyteArray array)
{
    //printf("This is MD_GetHsmInfo()\n");
    jboolean isCopy;
    jsize len = (*env)->GetArrayLength(env, array);
    jbyte* bp = (*env)->GetByteArrayElements(env, array, &isCopy);
    if (!bp) {
        // exception handling here
        return 0;
    }

    jint ret = MD_GetHsmInfo(hsmIndex, infoType, bp, len);
    int mode = 0;
    // if error code is returned then do not save changes
    if(ret != 0) {
        mode = JNI_ABORT;
    }
    (*env)->ReleaseByteArrayElements(env, array, bp, mode);

    return ret;
}

JNIEXPORT jint JNICALL Java_com_sprint_jni_JNISandbox_MD_1TestByteBuffer
    (JNIEnv *env, jobject obj, jint hsmIndex, jint infoType, jobject buf)
{
    jbyte *bp = (*env)->GetDirectBufferAddress(env, buf);
    jint len = (*env)->GetDirectBufferCapacity(env, buf);
    //printf("length of buffer = %d", len);
    jint ret = MD_GetHsmInfo(hsmIndex, infoType, bp, len);
    return ret;
}

Both functions seem to work fine, but I would like to make both functions production-ready, so that I can demonstrate without bias which approach to use. As I am not too familiar with JNI, it is likely that my code is not written optimally, so I would appreciate any feedback on that too.

Background

In the above snippet, MD_GetHsmInfo is a C function which my Java code needs to call.

MD_RV MD_GetHsmInfo(uint32_t hsmIndex, MD_Info_t infoType, void *pValue, uint32_t valueLen);

pValue is described as such:

  The address of the buffer to hold the information value. The buffer
  must have been allocated by the caller. The size of the buffer is
  determined by the infoType that is being obtained.

MD_RV and MD_Info_t are enums defined as follows:

typedef enum {
    MDI_HSM_DESCRIPTION = 1,
    //...so on
} MD_Info_t;

// similar implementation for MD_RV

The actual implementation of the function resides within a DLL file, but I do not have access to it yet. For now I have written a dummy implementation as follows:

int MD_GetHsmInfo(int hsmIndex, int infoType, void *pValue, int valueLen) {
    //printf("This is dummy implementation of MD_GetHsmInfo()\n");
    char *str;
    if(infoType == 1) {
        str = "info = 1";
    } else if(infoType == 2) {
        str = "info = 2";
    } else {
        str = "no info";
    }

    jbyte *jb = (jbyte*)pValue;
    while(*str) {
        *jb++ = *str++;
    }
    *jb = 0;
    valueLen = strlen(str);

    return 0;
}

Minor issues in MD_GetHsmInfo()

valueLen = strlen(str); serves no purpose below as the value assigned to valueLen is not used.

int MD_GetHsmInfo(int hsmIndex, int infoType, void *pValue, int valueLen) {
    ...
    valueLen = strlen(str);
    return 0;
}

Perhaps

int MD_GetHsmInfo(int hsmIndex, int infoType, void *pValue, int *valueLen) {
    ...
    *valueLen = strlen(str);
    return 0;
}

or

int MD_GetHsmInfo(int hsmIndex, int infoType, void *pValue, size_t *valueLen) {

Cast not needed

int MD_GetHsmInfo(int hsmIndex, int infoType, void *pValue, int valueLen) {
  ...
  // jbyte *jb = (jbyte*)pValue;
  jbyte *jb = pValue;


Encode-symbol for Huffman tree From the text:

  Exercise 2.68.  The encode procedure
  takes as arguments a message and a
  tree and produces the list of bits
  that gives the encoded message.

(define (encode message tree)
  (if (null? message)
      '()
      (append (encode-symbol (car message) tree)
              (encode (cdr message) tree))))

  Encode-symbol is a procedure, which
  you must write, that returns the list
  of bits that encodes a given symbol
  according to a given tree. You should
  design encode-symbol so that it
  signals an error if the symbol is not
  in the tree at all. Test your
  procedure by encoding the result you
  obtained in exercise 2.67 with the
  sample tree and seeing whether it is
  the same as the original sample
  message.

The textbook provides the following definitions:

(define (make-leaf symbol weight)
  (list 'leaf symbol weight))
(define (leaf? object)
  (eq? (car object) 'leaf))
(define (symbol-leaf x) (cadr x))
(define (weight-leaf x) (caddr x))

(define (make-code-tree left right)
  (list left
        right
        (append (symbols left) (symbols right))
        (+ (weight left) (weight right))))

(define (left-branch tree) (car tree))

(define (right-branch tree) (cadr tree))
(define (symbols tree)
  (if (leaf? tree)
      (list (symbol-leaf tree))
      (caddr tree)))

(define (weight tree)
  (if (leaf? tree)
      (weight-leaf tree)
      (cadddr tree)))

(define (decode bits tree)
  (define (decode-1 bits current-branch)
    (if (null? bits)
        '()
        (let ((next-branch
               (choose-branch (car bits) current-branch)))
          (if (leaf? next-branch)
              (cons (symbol-leaf next-branch)
                    (decode-1 (cdr bits) tree))
              (decode-1 (cdr bits) next-branch)))))
  (decode-1 bits tree))
(define (choose-branch bit branch)
  (cond ((= bit 0) (left-branch branch))
        ((= bit 1) (right-branch branch))
        (else (error "bad bit -- CHOOSE-BRANCH" bit))))
(define (adjoin-set x set)
  (cond ((null? set) (list x))
        ((< (weight x) (weight (car set))) (cons x set))
        (else (cons (car set)
                    (adjoin-set x (cdr set))))))

(define (make-leaf-set pairs)
  (if (null? pairs)
      '()
      (let ((pair (car pairs)))
        (adjoin-set (make-leaf (car pair)    ; symbol
                               (cadr pair))  ; frequency
                    (make-leaf-set (cdr pairs))))))

(define sample-tree
  (make-code-tree (make-leaf 'A 4)
                  (make-code-tree
                   (make-leaf 'B 2)
                   (make-code-tree (make-leaf 'D 1)
                                   (make-leaf 'C 1)))))

(define sample-message '(0 1 1 0 0 1 0 1 0 1 1 1 0))

(define (encode message tree)
  (if (null? message)
      '()
      (append (encode-symbol (car message) tree)
              (encode (cdr message) tree))))

And I wrote the following answer:

(define (member? x set)
  (not (equal? (member x set) false)))

(define (encode-branch symbol tree)
  (let ((left (left-branch tree))
        (right (right-branch tree)))
    (cond ((member? symbol (symbols left)) (list 0 left))
          ((member? symbol (symbols right)) (list 1 right))
          (else (error "Symbol is not a member of either left or right branches of the tree - can't encode" symbol tree)))))

(define (encode-symbol symbol tree)
  (if (leaf? tree) '()
      (let ((new-branch (encode-branch symbol tree)))
        (cons (car new-branch) (encode-symbol symbol (cadr new-branch))))))

How can I improve my solution?

Wouldn't it be easier to unpack the tree into a dictionary mapping each symbol to its corresponding bit string?  Then you could simply look up each symbol in the input to generate the corresponding output bits.

EDIT:
As suggested by syb0rg, here is an implementation (C#, I'm afraid -- my Lisp is far too rusty -- although it's almost pure).  The part pertaining to my suggestion above lives in the HuffmanCodes function at the end.

void Main()
{
    var corpus = @"Lorem ipsum dolor sit amet, consectetuer adipiscing elit, sed diam nonummy nibh euismod tincidunt ut laoreet dolore magna aliquam erat volutpat. Ut wisi enim ad minim veniam, quis nostrud exerci tation ullamcorper suscipit lobortis nisl ut aliquip ex ea commodo consequat.";
    var hcs = HuffmanCodes(corpus);
    Console.WriteLine(hcs);
}

Dictionary<char, int> Histogram(string corpus) {
    var hg = new Dictionary<char, int>();
    foreach (var x in corpus) {
        int f;
        hg.TryGetValue(x, out f);
        hg[x] = f + 1;
    }
    return hg;
}

class HuffTree {
    internal char? Sym; // Non-null iff this is a leaf node with no children.
    internal int Freq;
    internal HuffTree L;
    internal HuffTree R;
}

// Oh, for a priority queue.  This is *really* inefficient!
HuffTree HuffmanTree(string corpus) {
    var hg = Histogram(corpus);
    var hts = hg.Keys.Select(x => new HuffTree { Sym = x, Freq = hg[x] }).OrderBy(t => t.Freq).ToList();
    while (2 <= hts.Count) {
        var leasts = hts.Take(2).ToList();
        var l = leasts[0];
        var r = leasts[1];
        var newHt = new HuffTree { Freq = l.Freq + r.Freq, L = l, R = r };
        hts = hts.Skip(2).Concat(new HuffTree[] { newHt }).OrderBy(t => t.Freq).ToList();
    }
    return hts.First();
}

Dictionary<char, string> HuffmanCodes(string corpus) {
    var codes = new Dictionary<char, string>();
    Action<HuffTree, string> a = null; // Sweet recursion.
    a = (ht, code) => {
        if (ht.Sym != null) {
            codes[(char)ht.Sym] = code;
        } else {
            a(ht.L, code + "0");
            a(ht.R, code + "1");
        }
    };
    a(HuffmanTree(corpus), "");
    return codes;
}


Rehashing a hash table in c++ with quadratic probing The code below is my attempt at trying to create a hash table. I'm currently stuck with the rehash function as I think it's not efficient enough (I believe it's O(n^2). I'd be grateful if someone could give some comments and suggestions on how I could improve my rehash function.

class Hashtable{
private:
    int sze; //size: number of values are currently in the hashtable
    int cap; //capacity: the size of the hashtable

    struct HashNode{
        string value;
    };

    HashNode** arr;  //bucket

    //determine whether the number is prime or not
    bool IsPrime(int number){
        if (number == 2 || number == 3)
            return true;

        if (number % 2 == 0 || number % 3 == 0)
            return false;

        int divisor = 6;
        while (divisor * divisor - 2 * divisor + 1 <= number)
        {

            if (number % (divisor - 1) == 0)
                return false;

            if (number % (divisor + 1) == 0)
                return false;

            divisor += 6;

        }

        return true;

    }

    //find the next prime number that is >= a
    int NextPrime(int a){
        while (!IsPrime(++a)){ }
        return a;
    }

    int hashing(const string &s) const{
        int h = 0;
        for (int i = 0; i < s.size(); i++)
        {
            h += int(s[i]);
        }

        return h;
    }

    void rehashing ()
    {
        int oldCap = cap;
        sze = 0;
        //Doubling the capacity
        cap = NextPrime(cap*2);

        HashNode** oldArr = arr;
        arr = new HashNode*[cap]();

        //moving the values to the new after rehashing
        for (int i = 0; i < oldCap; i++){
            if (oldArr[i] != nullptr){
                for (int j = 0; j < cap; j++){
                    int index = (hashing(oldArr[i]->value) + j*j) % cap;
                    if (arr[index] == nullptr){
                        arr[index] = new HashNode {oldArr[i]->value};
                        sze++;
                        break;
                    } //if
                } //for
                delete oldArr[i];
                oldArr[i] = nullptr;
            } //if
        } //for

        delete[] oldArr;
    }

public:
    // Constructor
    Hashtable(int ini_cap = 101) : sze(0), cap(ini_cap), arr(new HashNode*[cap]){
        for (int i = 0; i < cap; i++)
        {
            arr[i] = nullptr;
        }

    }

    //Destructor
    ~Hashtable(){
        for (int i = 0; i < cap; i++){
            if (arr[i] != nullptr){
                delete arr[i];
                arr[i] = nullptr;
            }
        }
        delete[] arr;
    }

    double load_factor() const {return double(sze)/cap;}

    void put(const string& s){
        //Initialize a new node for the new input
        HashNode* temp = new HashNode{s};

        //Insert using quadratic probing
        for (int i = 0; i < cap; i++){
            int index = (hashing(s) + i*i) % cap;
            if (arr[index] == nullptr){
                arr[index] = temp;
                sze++;
                break;
            }
        }

        if (load_factor() > 0.5){
            rehashing();
        } //if

    } //add
 };

Ideally, I think i'd be the best If I could make it O(n). So if anyone have any idea on how I could do that please tell me. Thank you.

Rreplace the inner for loop of rehashing with a call to put. put has an average runtime of \$\mathcal{O}(1)\$ . So, rehashing has an average runtime of  \$\mathcal{O}(n)\$ since it is \$n\$ put operations.

It would look something like this:

void rehashing() {
  int oldCap = cap;
  sze = 0;
  cap = NextPrime(cap * 2);

  HashNode** oldArr = arr;
  arr = new HashNode*[cap]();

  for (int i = 0; i < oldCap; ++i) {
    if (oldArr[i] != nullptr) {
      put(oldArr[i]->value);
      delete oldArr[i];
    }
  }

  delete[] oldArr;
}

Also, it might be useful to refactor put to have a private overloaded put member function which accepts a HashNode. The public put would just allocate a HashNode and call the private put. Then, for rehashing, one could use the private put and wouldn't need to delete the previous HashNode. This would save memory allocation and deletions.


Sorting dictionary alphabetically I have this method which successfully sorts the incoming dictionary alphabetically:

private Dictionary<string, string> sortMacroDictionary(Dictionary<string, string> whichDictionary) {
    Dictionary<string, string> sortedDictionary = new Dictionary<string, string>();
    List<string> tempList = new List<string>();
    string outValue;

    if (whichDictionary.Count > 0) {
        foreach (var entry in whichDictionary) {
            tempList.Add(entry.Key);
        }

        tempList.Sort();

        foreach (string entry in tempList) {
            if (whichDictionary.TryGetValue(entry, out outValue)) {
                sortedDictionary.Add(entry, outValue);
            }
        }
    }

    return sortedDictionary;
}

It works as intended and I am not experiencing any issues with it. I'm just wondering if there is a way to improve this method? Maybe there are some issues with my current code that I haven't foreseen?

I'm afraid your sorting is in vain because the normal dictionary does not guarantee that the items will be enumerated in the same order as you added them:

  For purposes of enumeration, each item in the dictionary is treated as a KeyValuePair<TKey, TValue> structure representing a value and its key. The order in which the items are returned is undefined.

If they are, then this is a pure coincidence.

You'll be safe if you use the SortedDictionary instead which:

  Represents a collection of key/value pairs that are sorted on the key.

If we assume for a moment that a Dictionary<TKey, TValue> can be sorted the way you do it then it can be also achieved with a simple LINQ query:

private Dictionary<string, string> sortMacroDictionary(Dictionary<string, string> source)
{
    return source.Keys.OrderBy(k => k).ToDictionary(k => k, k => source[k]);
}

You first get all keys, order them, then you create a new dictionary based on the new key order. This is virtually the same as what you are doing now but much shorter.

As far as your code is concerned there is no need to check whichDictionary.Count > 0 becasue if the dictionary is empty your loops won't run. The same applies to this query.

My advice is to use the SortedDictionary. This dictionary maintains the order the entire time. It sorts itself so to speak, as you add/remove items. You don't have to sort it manually.


x86 program to do an exponent For continuity, my last question was with adding numbers together with a function call: Example x86-64 program. In this one I've tried to apply the lessons pointed out in the previous answer (stack alignment, using edx instead of rdx where the results will fit). I've tried to comment inline with the code:
# We are going to calculate 7^2 + 2^4 = 49 + 16 = 65
# The exponent function supports whole numbers (long) >= 0

.section .data
base_1: .long   7
exp_1:  .long   2
base_2: .long   2
exp_2:  .long   4

.section .text
.globl _start
_start:

    # We will do the first function call, 7^2
    mov base_1(%rip),   %edi
    mov exp_1(%rip),    %esi
    call exp

    # needs to be 16-byte aligned before the next function call so do two pushes
    pushq $0
    pushq %rax

    # Now do the second function call
    mov base_2(%rip),   %edi
    mov exp_2(%rip),    %esi
    call exp

    # We have the return value in %eax so let's add this with our previous function's value
    popq %rdi

    add %eax,   %edi
    mov $60,    %eax
    syscall

exp:
    # Initialize %eax to 1
    mov $1,     %eax

exp_op:
    cmp $0,     %esi
    je exp_ret
    imul %edi,  %eax
    dec %esi
    jmp exp_op

exp_ret:
    ret

I have a few specific questions about the code:

Is doing a raw push/pop common? Or is the convention always to do push rbp mov rsp rbp...pop rbp. Why is one method preferred over the other?
What if I wanted to store the result of the first calculation in a register: are there any registers that are guaranteed to be preserved between function calls? Or is this why the push...pop method is used so much?
Finally, does my exp seem ok? It feels a bit odd having three labels in what amounts to one function. How could that be improved?

I'm no expert, but what the heck, here's a review comment:
# needs to be 16-byte aligned before the next function call so do two pushes
pushq $0
pushq %rax

This comment is good, but I would rephrase it. Two eight-byte pushes makes 16 bytes and doesn't change the stack alignment. Therefore I infer that one of these pushes is significant and the other one is insignificant — but your comment doesn't tell me which is which! So you might say instead
# one extra push to preserve 16-byte stack alignment
pushq $0

# push the result of `exp`
pushq %rax

You could make the generated code smaller by eliminating the insignificant constant $0:
# push the result of `exp`, plus one extra push to preserve 16-byte stack alignment
pushq %rax
pushq %rax

Now the reader doesn't even need to figure out which push is the significant one, because both pushes do the same thing!

But why is preserving 16-byte alignment on calls important? That's not a requirement of the machine. You seem to be trying to follow some specific ABI, like maybe for interoperability with C or C++. Your external documentation should be clearer about what ABI you're trying to follow.
And then, if you are trying to interoperate with C code, you could improve your code by indicating which of its labels are meant as external entrypoints and which ones are just internal local labels. It seems like you intend exp to be called from other code — it's an entrypoint — but e.g. exp_op is not callable, and exp_ret is technically callable but just acts as a no-op. You might mark them somehow as "local implementation details, not for external consumption."
Yeah, you technically already do this by exporting .globl _start and not .globl exp — but there's still a big difference between the callable function exp and the local label exp_op which is not reflected in your naming scheme. If I were doing this, I'd add .globl exp and I'd rename exp_op, exp_ret to something like Lexp1, Lexp2 or L1_looptop, L2_loopend.


Create a collection of functions tupled with a collection of parameters, and execute the collection in order in Scala I am trying to create a collection of Unit functions that have any arity and type, and will execute them in order.

This is the working implementation that I have:

/** A Subroutine simply represents a unit function that takes any number of parameters. */
type Subroutine = (Any) => Unit

/** A Routine represents a collection of functions which may consume a collection of parameters and will always
  * complete an action.
  */
type Routine = List[(Subroutine, Seq[_])]

/** This function will consume a list of functions executing them in order.
  *
  * @param routine is the list of functions and their parameters to be consumed.
  */
def executeRoutine(routine: Routine): Unit = {
  routine.headOption match {
    case Some((f, x)) =>
      f(x)
      executeRoutine(routine.tail)
    case None => ()
  }
}

and when I declare a function or "subroutine", I have to declare it like this in order to extract the parameters:

def myFunction: Subroutine = {
  case (firstParam: Int) :: (secondParam: String) :: _ => ...
  case _ => throw new InvalidParameterException()
}

def myFunction2: Subroutine = {
  case (firstParam: Int) :: _ => ...
  case _ => throw new InvalidParameterException()
}

val routine: Routine = List(
  (myFunction, Seq(firstParam, secondParam)),
  (myFunction2, Seq(firstParam))
)

I realize that this is not close to idiomatic Scala code, and I've been working on implementing a different approach which will partially apply functions one parameter at a time, and then execute the function after the parameters are fully applied. I do not yet have a working version of this approach, but this is as far as I have gotten:

/** A Subroutine simply represents a unit function that takes any number of parameters. */
sealed abstract class Subroutine extends (Any => Unit)

case class Curried(partiallyApplied: Function[_, Function[_, _]], unapplied: Function[_, _]) extends Subroutine

case class Uncurried(function: Function[_, Unit]) extends Subroutine

/** A Routine represents a collection of functions which may consume a collection of parameters and will always
  * complete an action.
  * Using HList for the parameters, from Shapeless
  */
type Routine = List[(Subroutine, HList)]

/** This function will consume a list of functions executing them in order.
  *
  * @param routine is the list of functions and their parameters to be consumed.
  */
def executeRoutine(routine: Routine): Unit = {
  routine.headOption match {
    case Some((f, x)) =>
      executeSubroutine(f, x)
      executeRoutine(routine.tail)
    case None => ()
  }
}

private def executeSubroutine(subroutine: Subroutine, params: HList): Any = {
  subroutine match {
    case Curried(partiallyApplied, unapplied) =>
      partiallyApplied(params.head)(executeSubroutine(unapplied, params.tail))
    case Uncurried(function) => function(params.head)
  }
}

If anyone can help me out that would be greatly appreciated!

Unless I misunderstand your question, I think this is what you want:

type Subroutine[Param] = Param => Any

type Routine = Seq[(Subroutine[_], Seq[Any])]

def execRoutine(routine: Routine): Unit = routine foreach { case (sub, params) => execSubroutine(sub, params) }

def execSubroutine[SubParam] (subroutine: Subroutine[SubParam], params: Seq[Any]): Unit = {
    require(params.nonEmpty)

    subroutine(params.head.asInstanceOf[SubParam]) match {
        case sub: Subroutine[_] => execSubroutine(sub, params.tail)
        case () => require(params.tail.isEmpty) // Require correct number of params.
        case _ => throw new IllegalStateException()
    }
}

val add2 = (a: Int) => (b: Int) => println(a + b)
val add3 = (a: Int) => (b: Int) => (c: Int) => println(a + b + c)

execRoutine(List(
    (add2, List(3, 4)),
    (add3, List(3, 4, 5))
))

Try it online!

This program prints 7 and 12. Unfortunately, if the parameters are of the wrong type, a runtime exception occurs instead of a compile time one. Attempting to apply the less or more than the required parameters also results in a runtime exception.