Best way to do unknown number of nested v-for loops, or how to do while loops in VueJs (is there a "v-while"?) I want to recursively use a Vue component for each item in a multidimensional array, which is a list of topics. For example, Topic 1 can have subtopics 1.1, 1.1.1 and 1.1.2, but topic 2 might have no subtopics at all. (My working code is presented at the bottom)
My Array in Vue's app data is something like:
[
  ['Topic 1', [
    'Topic 1.1', [
       ['Topic 1.1.1', []],
       ['Topic 1.1.2', []]
    ]
  ],
  ['Topic 2', []],
  ['Topic 3', [
    ['Topic 3.1', []],
    ['Topic 3.2', []],
    ['Topic 3.3', []],
    ]
  ]
]

Supposing my component is just a details tag, this is my expected HTML output (borders were added just for clarity):

details > details {
  margin-left: 2em;
  border: 1px solid black;
}

Topic 1
  Topic 1.1
    Topic 1.1.1

    Topic 1.1.2

Topic 2

Topic 3
  Topic 3.1

  Topic 3.2

  Topic 3.3

etc.

I can't anticipate all v-for loops I'll use, because I don't know the depth of each topic (i.e., I don't know how many subtopics there are, it depends on each topic and there can be dozens or none at all).
My solution was to use a v-if to check if there are subarrays inside the current array; if there are, a component is created for the subarrays too (and the component's template includes in the end this div with a v-if, so it works recursively).
This works, but looks clunky. So my question is: what is the best way to run a while-loop with Vue (for the cases where I don't know how many for-loops I need to run)? Or is my solution already the best way to approach this?
(I first thought of creating a method, and calling this method in the end to check if there are subarrays and then create components for them basically via JavaScript, but this seems worse than using v-if and adding a v-for for adding components to the subarray items)

Here's my working code:

Vue.component('details-component', {
  template: ` {{ arritem[0] }}

`,
  props: {
    arritem: {
      type: Array,
      required: true
    }
  }
});

const app = new Vue({
  el: "#root",
  data: {
    arr: [
      ['Topic 1', [
        ['Topic 1.1', [
          ['Topic 1.1.1', []],
          ['Topic 1.1.2', []]
          ]
        ]
      ]],
      ['Topic 2', []],
      ['Topic 3', [
        ['Topic 3.1', []],
        ['Topic 3.2', []],
        ['Topic 3.3', []],
      ]]
    ]
  }
});
details details {
  margin-left: 2em;
}

There is no v-while, no. You can however use v-for over a computed property (which you can compute using while if you so wish).
Although I don't see a reason to use any while-loop. What you have is a data structure with topics and subtopics of any arbitrary length. Using a recursive component for this, which you have done here, is a good choice.
About the only thing that I would change is to, instead of using arritem[0] and arritem[1], using an object with properties name and children. This can possibly also help you determine if a component has any children or not.


Simple singly linked list implementation in C I have implemented a LinkedList with the ability to add, get elements from a particular position and ability to show all the elements in the array in to the console (main purpose to implement this was to ensure that all the elements are being populated in the LinkedList correctly).

I would highly appreciate comments based on:

The efficiency of my code
Conceptual misinterpretations of a linked list, if any
Violations of best practices
Better way of achieving this

This is the code I have written:

#include <stdio.h>
#include <stdlib.h>

struct Element {
    int value;
    struct Element *nextElement;
};

struct LinkedList {
    struct Element *firstElement;
};

void add(struct LinkedList *list, int value) {
    if(list->firstElement == NULL) {
        struct Element *newElement;
        newElement = malloc(sizeof(struct Element));

        newElement->value = value;
        newElement->nextElement = NULL;

        list->firstElement = newElement;
    } else {
        struct Element *lastElement;

        lastElement = list->firstElement;

        while(lastElement->nextElement != NULL) {
            lastElement = lastElement->nextElement;
        }

        struct Element *newElement;
        newElement = malloc(sizeof(struct Element));

        newElement->value = value;
        newElement->nextElement = NULL;

        lastElement->nextElement = newElement;
    }
}

int getElement(struct LinkedList *list, int index) {
    int iteratedIndex = 0;
    int returnValue = 0;

    struct Element *temporaryElement = list->firstElement;

    while(temporaryElement != NULL) {
        if(iteratedIndex == index) {
            returnValue = temporaryElement->value;
            return returnValue;
        }

        iteratedIndex = iteratedIndex + 1;
        temporaryElement = temporaryElement->nextElement;
    }

    return 0;
}

void showAllElements(struct LinkedList *list) {
    struct Element *lastElement = list->firstElement;

    while(lastElement != NULL) {
        printf("%d\n", lastElement->value);
        lastElement = lastElement->nextElement;
    }
}

int main()
{
    struct LinkedList list = {NULL};
    add(&list, 5);
    add(&list, 10);
    add(&list, 15);
    showAllElements(&list);

    int value = getElement(&list, 0);

    printf("%d\n", value);

    return 0;
}

Here's the output I got:

5
10
15
5

Which is correct, first three listing all the elements in the LinkedList and last one fetching the element in the 0th index.

In terms for efficiency and concept, the implementation is fine.
It lacks the more interesting functionality such as deleting and inserting items.

The most important missing element is deleting the linked list itself:
when you allocate memory using malloc,
it's important to add functions to clean that up at some point.

Code duplication

In addElement,
the logic of creating a new element is duplicated twice.
This could be easily avoided by moving that logic to another function, for example:

struct Element * createElement(int value) {
    struct Element *newElement;
    newElement = malloc(sizeof(struct Element));

    newElement->value = value;
    newElement->nextElement = NULL;
    return newElement;
}

On closer look,
you didn't even really need an extra method,
you could eliminate the duplicated logic by moving the element creation before the if condition:

void add(struct LinkedList *list, int value) {
    struct Element *newElement = createElement(value);
    if(list->firstElement == NULL) {
        list->firstElement = newElement;
    } else {
        // ...
        lastElement->nextElement = newElement;
    }
}

Naming

Most of the names are good and nicely descriptive.
But there are a few exceptions:

getElement: despite its name, this function returns an int, not an Element. So getValue would be better.
lastElement as loop is not so great, as it's typically not referencing the last element. node would be a more common name for the purpose

Trivial simplifications

Some of the functions can be simplified very easily.

For example,
in getElement you really don't need the returnValue variable.
You assign to it right before you return it,
and that's the only use.
So avoid the pointless local variable.
Every variable has the potential of getting misused.
So if a variable has no good purpose to serve, then don't use it.

Another minor point is that instead of iteratedIndex = iteratedIndex + 1 you can write simply ++iteratedIndex.


Help me condense a series of lengthy javascript conditional arrays I'm not too used to working with arrays in javascript, so was wondering if there is a good way to condense this down.

Any pointers much appreciated!

you can see it in action @ http://www.productionlocations.com/locations

if($('.filters').length) {
    if(url === 'http://www.productionlocations.com/locations' || url === 'http://www.productionlocations.com/locations/') {
        searchval[0] = 'Find a location…';
        searchval[1] = 'Try searching by zip code, like 90012…';
        searchval[2] = 'Know your location ID? Find it like so: #xxxx';
        searchval[3] = 'Search locations by type, like apartment, office, or retail...';
        searchval[4] = 'Search locations by area, like downtown, Pasadena, or Hollywood...';
        searchval[5] = 'Search locations by style, like art deco, tudor, or industrial...';
        searchval[6] = 'Search locations by feature, like fireplace, pond, or tiled floors...';
    } else {
        searchval[0] = 'Modify your search by type, like apartment, office, or retais...';
        searchval[1] = 'Modify your search by area, like downtown, Pasadena, or Hollywood...';
        searchval[2] = 'Modify your search by style, like art deco, tudor, or industrial...';
        searchval[3] = 'Modify your search by feature, like fireplace, pond, or tiled floors...';
        searchval[4] = 'Modify your search…';
        searchval[5] = 'Modify your search by zip code, like 90012…';
    }

    searchkey = Math.floor(Math.random() * searchval.length);
    $('#s').val(searchval[searchkey]);
} else {
    searchval[0] = 'Find a location…';
    $('#s').val(searchval[0]);
    inputReplace();
}

Assuming the variables you're using aren't needed later, you can condense it down to this:

var messages = [
    'Find a location…',
    'Try searching by zip code, like 90012…',
    'Know your location ID? Find it like so: #xxxx',
    'Search locations by type, like apartment, office, or retail...',
    'Search locations by area, like downtown, Pasadena, or Hollywood...',
    'Search locations by style, like art deco, tudor, or industrial...',
    'Search locations by feature, like fireplace, pond, or tiled floors...',
    'Modify your search…',
    'Modify your search by zip code, like 90012…'
], messageSets = [
    [3, 4, 5, 6, 7, 8],
    [0, 1, 2, 3, 4, 5, 6],
    [0]
];

var s = messageSets[$('.filters').length ? +/^\/locations\/?$/.test(location.pathname) : 3];
$('#s').val(messages[s[Math.random() * s.length | 0]]);
inputReplace();

Ideally the first var statement would be at a higher level, so it would only be run once, not every time the function is executed.

You could shorten it even further by using ranges instead of sets, but that could get complicated if you want to add more messages later. On the flip side, if you are planning to add a lot more messages, it would probably be clearer to make messages and messageSets objects (= dictionaries), so that you can name each message and set.

edit: here's how you would do the same thing using objects instead of arrays. The only difference is that each message, and set, has a name instead of a number, which makes references to it clearer to read.

var messages = {
    default: 'Find a location…',
    zip: 'Try searching by zip code, like 90012…',
    id: 'Know your location ID? Find it like so: #xxxx',
    type: 'Search locations by type, like apartment, office, or retail...',
    area: 'Search locations by area, like downtown, Pasadena, or Hollywood...',
    style: 'Search locations by style, like art deco, tudor, or industrial...',
    feature: 'Search locations by feature, like fireplace, pond, or tiled floors...',
    modify: 'Modify your search…',
    modify_zip: 'Modify your search by zip code, like 90012…'
}, messageSets = {
    locations: 'default zip id type area style feature'.split(' '),
    filters: 'type area style feature modify modify_zip'.split(' '),
    default: ['default']
};

var s = messageSets[$('.filters').length ?
    /^\/locations\/?$/.test(location.pathname) ? 'locations' : 'filters' :
    'default'
];
$('#s').val(messages[s[Math.random() * s.length | 0]]);
inputReplace();

Note that this has the additional advantage of making the order of the objects not matter: you could add alert: 'Enter something!', directly under the first line and it wouldn't break anything.


Are your servers down...? If you came here because of the title, chances are your server isn't down. But if by some miraculous reason it is:
A while ago, I posted a program that would ssh and ping all the servers at work. I got some very good feedback and went ahead and rolled with it.
Here's what I came up with:

It now is much more readable, in my opinion
Pings the servers after SSHing into them
Then if it finds one of the servers to be offline, or errors out connecting, writes data to a .txt file containing the server name, and the user who deployed the program.

I of course will be leaving some information off due to security.
Source code:
#!/usr/local/bin/ruby

require 'rubygems'
require 'net/ssh/gateway'
require 'work/mail/gem'
require 'etc'

def mail_error(e)
  x = AD::Mail.new
  x.from        = "email@email.email"
  x.to          = "email@email.email"
  x.subject     = "Server Loading Error"
  x.body        = e.message
  x.send
end

def scanning_all_servers(server)
  begin
    check = Net::SSH.start(server, @username, :password => @password)
    cmd = "ping -c 1 #{server}"
    check.exec!(cmd)
  rescue => e
    mail_error(e)
    :error
    write_data(server)
  end
end

def server_status(server)
  result = scanning_all_servers(server)

  message_server_online = /1 received, 0% packet loss/i
  message_server_offline = Regexp.union('0 recieved, +1 errors', 'Connection closed')

  result =~ message_server_online  ? :online  :
  result =~ message_server_offline ? :offline : :error
end

def status_message(server, status)
  status == :online  ? "server online: #{server}"  :
  status == :offline ? "[#{Time.now}] SERVER OFFLINE: #{server}" :
                       "[#{Time.now}] Error loading server: #{server}."
end

def write_data(server)
  x = "[#{Time.now}]: Server: #{server} down. Checked by #{@username}. ------------"
  if File.exist?("path/to/log")
     File.open("path/to/log", "a+"){ |s| s.write(x) }
  else
    new_file = File.new("path/to/log", "w")
    new_file.puts(x)
    new_file.close
  end
end

@username = Etc.getlogin
@password = nil

server_list = %w(all of our servers)

server_list.each do |server|
  status = server_status(server)
  puts status_message(server, status)
end

Example of usage:

server online: server
server online: server
SERVER OFFLINE: server
server online: server
server online: server
server online: server
server online: server
server online: server
Error loading server: server
server online: server
server online: server
server online: server
server online: server
server online: server
server online: server
server online: server
server online: server
server online: server
server online: server
server online: server

Example of log:

[2015-12-14 06:11:41 -0600]: Server: server down. Checked by user. ------------
[2015-12-14 06:11:45 -0600]: Server: server down. Checked by user. ------------

What I would like to know:

Is there any better ways to write data to files?
Is there also more readable syntax to use here?

I'll proceed top to bottom.

mail_error

Which would you rather read: x.body = e.message or mail.body = exception.message?  Don't try to save a few characters on naming — your fellow programmers will like you better.  In particular, x has a connotation of being a floating point number, which this isn't.

scanning_all_servers and server_status

Net::SSH#start does not return a "check".  Rather, it returns a connection.  conn = Net::SSH.start(…) would be more appropriate.  Even better, you should call start with a block so that the connection gets closed without an explicit call to close.

As noted in a review of your previous question, scanning_all_servers should be named scan_server, since it acts on just one server.  The :error symbol isn't doing anything.  Either move it to the end as the implicit return value, or get rid of it.

However, the way that server_status and scanning_all_servers cooperate is weird.  scanning_all_servers plays a part in interpreting the result, but only does a partial job.  Yet, it tries to log that result anyway.  It is also odd that the codepath for writing to the screen is so different from the codepath for writing to the log file.  What you want is one function whose job is to scan the server and produce a status code.  Everything else belongs in some other function.

status_message and write_data

status_message would look slightly tidier using a case.

write_data has a File.exist? test for no reason.  Mode 'a+' will create the file if it doesn't already exist.  But why does the append case call write instead of puts, so that there is no newline written?  Also, minor note: s looks like a string, so prefer f as the variable name for a file.

You seem to be more careful about logging failures than successes, but the successes are just as important, and should also be written to the log file.  Also, all results, whether printed to screen or to the log file, should have timestamps.

Suggested solution

require 'rubygems'
require 'net/ssh/gateway'
require 'work/mail/gem'
require 'etc'

def mail_error(exception)
  mail = AD::Mail.new
  mail.from     = 'email@example.com'
  mail.to       = 'email@example.com'
  mail.subject  = "Server down"
  mail.body     = exception.message
  mail.send
end

def scan_server(server)
  begin
    Net::SSH.start(server, @username, :password => @password) do |ssh|
      case ssh.exec!("ping -c 1 #{server}")
      when /1 received, 0% packet loss/i
        :online
      else
        :offline
      end
    end
  rescue => e
    :error
  end
end

def status_message(server, status)
  sprintf("[%s]: %s\n", Time.now,
    case status
    when :online
      "server online: #{server}"
    when :offline
      "SERVER OFFLINE: #{server}"
    else
      "Error connecting to server: #{server}"
    end
  )
end

def log(message)
  File.open('path/to/log', 'a+') { |f| f.puts(message) }
end

@username = Etc.getlogin
@password = …

server_list = %w(all of our servers)

server_list.each do |server|
  status = scan_server(server)
  message = status_message(server, status)
  log message
  puts message
end


Transforming data from an API using Typescript and Lodash As a front-end developer, I am given the following result from an API, an array of objects that looks like:
[{
    Color: "#065b79",
    GroupName: "VXYZ",
    QuoteCount: 4,
    SortOrder: 0,
    TabCategoryID: "CHSI1",
    TabCategoryName: "Workers' Compensation",
    TabCategoryOrder: 0,
    TabID: 1,
    TabName: "new"
}]

I need to display the data in a more hierarchical/nested format:
[{
    GroupName: 'VXYZ',
    TabCategories: [
        {
            TabCategoryID: 'CHSI1',
            TabCategoryName: "Workers' Compensation",
            TabCategoryOrder: 0,
            Tabs: [
                {
                    Color: '#065b79',
                    SortOrder: 0,
                    TabID: 1,
                    TabName: 'new',
                    QuoteCount: 4
                }
            ]
        }
    ]
}]

Any data in an array can have any number of array items. In the models above, I just use a single instance of each array item.
This is the code I used to format the data
I'm using Typescript and lodash. The API is experimental, and our policy is to just use the any type for any data coming from experimental APIs. This explains the (Quotes: any) — it is the data from the API.
public FormatQuoteSummaries(Quotes: any): any {

    return _(Quotes)
    .map(Quote => Quote.GroupName)
    .uniq()
    .map(UniqueGroupName => {
        const GroupName = UniqueGroupName
        const AllTabCategories = _.filter(Quotes, (Quote: any) => {
            return Quote.GroupName === UniqueGroupName
        })
        const TabCategories = _(AllTabCategories).map(TabCategory => {
            return {
                TabCategoryID: TabCategory.TabCategoryID,
                TabCategoryName: TabCategory.TabCategoryName,
                Tabs: _.filter(Quotes, (Quote: any) => {
                    return Quote.TabCategoryName === TabCategory.TabCategoryName &&
                        Quote.GroupName === TabCategory.GroupName
                })
            }
        })
            .uniqBy('TabCategoryID')
            .value();

        return {
            GroupName,
            TabCategories
        }
    })
    .value();
}

First and most importantly, change your policy about using any for unstable APIs.

Typescript is an incredibly powerful tool for making it possible to work with constantly changing APIs. When you use any you are throwing away (nearly) all the benefits which Typescript gives you. If you declare an interface for the data being returned by the API, you can simply modify the interface when the format changes and you will be alerted to any code which needs to be updated. If you instead use any your code will compile without any warnings - and then break in production.

Besides this, there really isn't that much to mention. Good work!

On to the code:

The first thing I did was add types, here are the interfaces I will be using for the rest of the answer:

interface APITabCategory {
    Color: string;
    GroupName: string;
    QuoteCount: number;
    SortOrder: number;
    TabCategoryID: string;
    TabCategoryName: string;
    TabCategoryOrder: number;
    TabID: number;
    TabName: string;
}

interface Tab {
    Color: string;
    SortOrder: number;
    TabID: number;
    TabName: string;
    QuoteCount: number;
}

interface TabCategory {
    TabCategoryID: string;
    TabCategoryName: string;
    TabCategoryOrder: number;
    Tabs: Tab[]
}

interface TabCategoryCollection {
    GroupName: string;
    TabCategories: TabCategory[]
}

It doesn't do what you think it does. As soon as I added interfaces for your types Typescript yelled at me that TabCategoryOrder was missing - this may be intentional as you are now using an array, but it is included in your sample output.
The current code includes a bunch of extra data in each Tab, this is fine so far as Typescript is concerned, but since this code normalizes the data you may want to consider removing the extra properties.
JavaScript has built in map and filter functions on arrays, I prefer using these to their lodash alternatives where appropriate. In this case, it removes most of your code's dependence on lodash.
Avoid double checking conditions. If you save the quotes with the current group name in an array, you can avoid checking the group name when extracting the tabs.
Get a linting program and use it - the current code is inconsistent with whether or not to use semicolons.

Here is how I would implement this function:

function FormatQuoteSummaries(Quotes: APITabCategory[]): TabCategoryCollection[] {
    const UniqueGroupNames = _.uniq(Quotes.map(Quote => Quote.GroupName));

    return UniqueGroupNames.map(GroupName => {
        const GroupQuotes = Quotes.filter(Quote => Quote.GroupName == GroupName)

        const TabCategories: TabCategory[] = GroupQuotes
            .filter(Quote => Quote.GroupName === GroupName)
            .map(({ TabCategoryID, TabCategoryName, TabCategoryOrder }) => {
                return {
                    TabCategoryID,
                    TabCategoryName,
                    TabCategoryOrder,
                    Tabs: GroupQuotes.filter(Quote => Quote.TabCategoryName === TabCategoryName)
                };
            });

        return {
            GroupName,
            TabCategories: _.uniqBy(TabCategories, 'TabCategoryID')
        };
    });
}

One last small nitpick, it is fairly standard is JavaScript / TypeScript to use camelCase as opposed to PascalCase for local variables and properties. Most JS / TS devs, when seeing Quotes expect a class / interface / enum / namespace, not an object or array. Feel free to disregard if your team has an established naming convention already.


Automating easy-rsa cert creation with Bash script In order to make cert creation with easy-rsa as practical for others, this script was created and it works so far.  The questions are:

where can it be improved?
are there flaws that are not obvious to the author?

The commentary is a bit extensive, but whomever will come across this in the future should be able to understand what's being done no matter how advanced.

#!/bin/bash
##
## This script automates the complete process of creating new easy-rsa  client certificates.
## It includes:     1. Mounting a block device.
##          2. Generating certificates using individual timestamps.
##          3. Copying the created certs to the mounted block device.
##
##
#
# First we look for the usb device at sdc1 and mount it. If not present tell the pebcac the it should insert one.

# all the predifined variables will end up beneath

myself=auto_rsa_gen.sh # tells the script who it is

export easy_rsa=../rsa_testing # tells the script where it is and exports this information for use in child processes

now=$(date +"%F")

##
# Show a list of available drives and ask the user for the dev he wants to mount
echo "These \n$(ls /dev | grep sd) \nare the drives available. Please tell me which one you want to mount \n(hint: its usually the one having just the  one additional entry. Like sdc1):"

# read user input (blkdev is the variable that will hold the user inuput for further processing)
read blkdev

##
# I. Mount
# check if de given device exists
if [ -b /dev/"$blkdev" ]
        then # if it exists mount it to /media
            sudo mount /dev/$blkdev /media
            echo "\nDevice $blkdev has been mounted!"
        else # if it doesn't exist
            echo "Device doesn't appear to exist! Restarting"
            sh "$myself" # restart the script
        fi

# Ask for an identifier to be used in the wrapper for easy-rsa cert creation
echo "\nPlease provide an identifier for the certs that will be created \n(The identifier will be appended to the current date of the machine):"

# read user input (ident is the variable that will hold the user input for further processing)
read ident

# Ask for ne number of certs to be created
echo "\nHow many certificates do you want me to create?"
# read user input (cnum is the variable that will hold the user input for further processing)
read cnum

##
# II. Wrapper for easy-rsa cert creation
#

# this is a while loop in case there was no input for ident. It only stop when the user hast put in something other than nothing
while [ -z $ident ]
    do
    echo "\nYou did not supply an identifier. Please do so or i will terminate."
    read ident # read user input again
done

# change directory to the easy-rsa directory
cd $easy_rsa

# tell user what you are going to do
echo "\nCreating $cnum certificates with ${now}_$ident."

# load vars file
. ./vars

##
# II.a Generation
# for loop that creates $cnum certificates and appends an incrementing number to the end of every
# file name

for i in $(seq -f %03g $cnum)
    do
        ${easy_rsa}/./build-batch ${now}_${ident}_$i

done

##
# II.b Copying
# for loop that searches for all certs created with this run and copies them to /media where $blkdev is mounted
find ${easy_rsa}/keys/ -name ${now}_${ident}*.crt -o -name ${now}_${ident}*.key > /media/${now}_${ident}
for f in $(find ${easy_rsa}/keys/ -name ${now}_${ident}*.crt -o -name ${now}_${ident}*.key )
    do
        cp $f /media

done
echo "\nCopied \n$(find ${easy_rsa}/keys/ -name ${now}_${ident}*.crt -o -name ${now}_${ident}*.key) \nto /media"

# unmounting $blkdev at /media
umount /media
echo "\nUmounted $blkdev at /media. Exiting! Bye!"ere

This

myself=auto_rsa_gen.sh # tells the script who it is

is better done like this:

myself="$0"

or, if you want just filename, then

myself="$(basename "$0")"

because it will give you the actual name of the script, instead of being hard-coded. This also opens a nice renaming possibilities if you use symlinks.

This part:

echo "These \n$(ls /dev | grep sd) \nare the drives available. Please tell me which one you want to mount \n(hint: its usually the one having just the  one additional entry. Like sdc1):"

# read user input (blkdev is the variable that will hold the user inuput for further processing)
read blkdev

##
# I. Mount
# check if de given device exists
if [ -b /dev/"$blkdev" ]
        then # if it exists mount it to /media
            sudo mount /dev/$blkdev /media
            echo "\nDevice $blkdev has been mounted!"
        else # if it doesn't exist
            echo "Device doesn't appear to exist! Restarting"
            sh "$myself" # restart the script
        fi

I think is better put in a while loop, instead of restarting a script, like this:

while true; do
  echo "Pick device, blah, blah,... Press Ctrl+C to abort."
  read blkdev
  if [ ! -b /dev/"$blkdev" ]; then
    echo "The device doesn't appear to exist!"
    continue;
  fi
  sudo mount /dev/"$blkdev" /media
  echo "\nDevice '$blkdev' has been mounted!"
done

You might also want to check if the mount command succeeded.

Also, /media is a standard directory for dynamic mounts. That means that your script will hide mountpoints of mounted CDs, DVDs, USB keys,... while it is running, which may be very inconvenient in some situations (for example, if you have some copying done in the background).

Btw, echo 'Line 1\nLine 2' outputs Line 1\nLine 2 instead of two lines (GNU bash, version 4.2.45, Fedora 19). The correct way for this is

echo -e 'Line 1\nLine 2'

I assume you have -e aliased, or on your distribution it is on by default, or something like that. However, it is not portable, so I suggest you add -e where needed.

I also have a bit of a problem with

cd $easy_rsa

These are my concerns:

It has a relative path to the subdirectory. This will hardly produce satisfactory results if your script is invoked from a directory other than you intended it to be invoked from. Maybe this value should either be prompted (with read), or picked up from the command line?
You're not checking that the directory exists. You could force the directory creation (mkdir -p "$easy_rsa"), and - on top of that - check that $0 is true (i.e., equal to 0) afterwards.

In the last line

echo "\nUmounted $blkdev at /media. Exiting! Bye!"ere

This ere in the end seems to be here by an error.

I also have one general remark. Instead of writing echo $something, cd $dir, and similar commands, I suggest you use echo "$something", cd "$dir", etc. There is no point in thinking if one of these may contain space and, when editing your script later on, thinking about fixing this if you do include a space. It's much easier and safer to always behave as if a variable can contain a space and just include the quotation marks.

Also, without the quotation marks, you're in trouble with all the variables that get their values from the user. The example for this is your $blkdev variable which you sometimes enclose in the quotation marks, but sometimes you do not (I added them in the mount command in my loop example above).


Project Euler #10 in C++ (sum of all primes below two million) Project Euler Problem 10 asks for the sum of all the primes below two million.

I'm creating a list of primes and keep checking any new number for divisibility with only prime numbers. I thought it should take less time but it is going over 4 minutes. How should I approach such a problem?

#include<iostream>
#include<list>

#define X 2000000

using namespace std;

list<long long> lst={2ll,3ll};

bool divideByAnyPrime(long long a)// checks if the number a is divisible by any prime in the list (lst)
{

    for( auto x:lst )
        if( a%x == 0 )
            return true;
    return false;
}

long long getNextPrime()//adds next prime number to list
{

    long long a=lst.back()+2;
    for( ; divideByAnyPrime(a); a+=2);
    lst.push_back(a);

    return a;

}

int main()
{
    long long sum=5ll;//sum of already existing elements 2 and 3
    while(lst.back()<=X)
    {
        sum+=getNextPrime();
    }
    sum-=lst.back();
    lst.pop_back();//as our list will contain one element at the last which we don't want

    cout<<"\nSum is "<<sum;

}

Here are a number of things that may help you improve your program.

Use a better algorithm

As already mentioned in the comments, a Sieve of Eratosthenes is going to be much, much faster than the current method.  Generally speaking, doing division is a computationally expensive operation.  For example, a simple Sieve program to solve this same problem takes 0.7 seconds on my machine, while your existing program takes almost 5 minutes (4:58.4).  However, we can still make considerable improvements without changing the basic algorithm you've already implemented.  The rest of the review concentrates on helping you improve your existing code and algorithm.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Consider improving names

I think divideByAnyPrime is not a terrible function name (although I'd probably call it isComposite), but X and lst are very poor variable names.  A good name should suggest something about what it contains.  For example, I've renamed X to MAXNUM in the next suggestion.

Prefer constexpr to old-style #define

Rather than using a #define for X the code could use a constexpr:

constexpr long long MAXNUM{2000000};

The advantage is that the value has an associated type.

Eliminate global variables where practical

The code declares a global variable to contain the list of primes.  Global variables obfuscate the actual dependencies within code and make maintainance and understanding of the code that much more difficult.  It also makes the code harder to reuse.  For all of these reasons, it's generally far preferable to eliminate global variables.  For MAXNUM, simply put it at the top of main.  For the list of primes, use the next suggestion.

Encapsulate functions and data within a class

Since you're writing in C++, I'd suggest making a class.  Here's the class declaration:

class Primes final : public std::list <long long>
{
public:
    Primes()
        : std::list<long long>{2,3}
    {
        for (auto n = back()+2; n <= MAXNUM; n+=2) {
            if (isPrime(n)) {
                push_back(n);
            }
        }
    }
private:
    static constexpr long long MAXNUM{2000000};
    bool isPrime(long long a) const;
};

Notice that MAXNUM is now a member of the Primes class and that the function isPrime is used instead of divideByAnyPrime.  Its sense is also inverted such that it only returns true if the passed number is prime.

Think carefully about the mathematics

Within the routine that check for divisibility by any existing prime, there's no need to go all the way to the end of the array.  If a number \$N\$is composite, its smallest factor must be \$\le \sqrt{N}\$.  This suggests that one way to write an isPrime function would be this:

bool Primes::isPrime(long long a) const
{
    long long sqrta = std::sqrt(a);
    for (auto it = ++begin(); *it <= sqrta; ++it)
        if( a % (*it) == 0 )
            return false;
    return true;
}

Note that this also skips the first item in the list, which we know is 2.

Use standard functions

Calculating the sum is actually easily done using a standard generic algorithm:

int main()
{
    Primes primes;
    long long sum = std::accumulate(primes.begin(), primes.end(), 0ll);
    std::cout << "Sum is " << sum << "\n";
}

Results

With all of these changes, the time drops to 1.14 seconds on my machine, which is a considerable improvement over the original 5 minutes.


Insertion Sort and Selection Sort Implementation I wanted to practice using templates since I have no experience with them, so I implemented these sorting algorithms.

Selection Sort:

template<typename Container>
void selectionSort(Container& numbers)
{
    for (auto iter = std::begin(numbers), iterEnd = std::end(numbers); iter != iterEnd; ++iter)
    {
        auto minNum = minIndex(numbers, iter, iterEnd);

        std::swap(*iter, *minNum);
    }
}

template<typename Container, typename Iter>
Iter minIndex(const Container& numbers, Iter start, Iter end)
{
    Iter minIdx = start;
    auto minNum = *start;

    while (++start != end)
    {
        if (*start < minNum)
        {
            minNum = *start;
            minIdx = start;
        }
    }
    return minIdx;
}

Insertion Sort:

template<typename Container>
void insertionSort(Container& numbers)
{
    for (auto iter = std::begin(numbers) + 1, iterEnd = std::end(numbers); iter != iterEnd; ++iter)
    {
        if (*iter < *(iter-1))
        {
            resort(numbers, iter);
        }
    }
}
template<typename Container, typename Iter>
void resort(Container& numbers, Iter containerIter)
{
    auto temp = *containerIter;

    while (containerIter != std::begin(numbers) && temp < *(containerIter-1))
    {
        *containerIter = *(containerIter - 1);
        --containerIter;
    }

    *containerIter = temp;
}

First tip: use more aggressive warnings, this way you would be warned by the compiler about passing unused parameters into functions, that is you would automatically made:

template<typename Iter>
Iter minIndex(Iter start, Iter end)

Another thing would be, to write your algorithms in similar way as the standard ones. That is avoid passing the reference to the container, but rather pass iterators into it. So your sort functions should look similarly to:

<typename InputIt>
void someSort (InputIt first, InputIt last)

This will e.g. allow you to sort specific parts of your container with no extra effort.


Generate 16-digit unique code (like product serial) I am not sure what to call this type of code (16-digit alphanumeric code).

Conditions are like this:

Digits (or characters) are limited 16
Numeric, alphabet mixed
No duplication allowed (not yet implemented)

One class with the main method:

public class QuadraDigitCipherGenerator {
    // Step 1. Generate 16 digit wich is not duplicated
    // Step 2. divide by 4 digit

    private static List<Integer> pickRandomIndex (String original, int count) {
        List<Integer> randIndices = new ArrayList<Integer>();
        Set<Integer> noDuplicateIndices = new HashSet<Integer>();
        System.out.println(noDuplicateIndices.size() + "    " + count);
        while (noDuplicateIndices.size() < count){
            System.out.println(noDuplicateIndices.size() + "    " + count);
            int randNumber = (int)(Math.random() * original.length());
            noDuplicateIndices.add(randNumber);
        }
        randIndices.addAll(noDuplicateIndices);
        return randIndices;
    }

    // FROM 97 - 122
    private static char convertDigit2Char(char digit) {
        return (char)(17 + digit);
    }

    private static String generateCode(int digit, int charCnt) {
        long nano = System.nanoTime();
        StringBuilder util = new StringBuilder(String.valueOf(nano));
        util = util.reverse();
        List<Integer> convertIndices = pickRandomIndex(util.toString(), charCnt);
        System.out.println("Below indices will be casting to Alphabet " + convertIndices);

        for(Integer i : convertIndices){
            char target = util.charAt(i);
            util.setCharAt(i, convertDigit2Char(target));
        }
        return util.toString();
    }

    private static String beautifyDigits(String original, int term){
        System.out.println("original " + original);
        return original.substring(0, term) + "-" + original.substring(term, 2*term)
                + "-" + original.substring(2*term, 3*term) + "-" + original.substring(3*term, 4*term);
    }

    public static void main(String argv[]) throws Exception {
        int digit = 16;
        System.out.println(digit + " digits random code > " + beautifyDigits(generateCode(16, 4), 4));
    }
}

The output looks like D33J-H872-3545-71A1, a product serial key.

Anyway, I think this code is not satisfied with the third condition (no duplication). Do you have any advice with regards to that aspect of the requirements?

Okay first off, let's make your random character choosing better.

The simplest way to get random symbols is to simply define the alphabet and then index into it.

So here's how you'd typically do it:

static final private String ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
final private Random rng = new SecureRandom();

char randomChar(){
    return ALPHABET.charAt(rng.nextInt(ALPHABET.length()));
}

String randomUUID(int length, int spacing, char spacerChar){
    StringBuilder sb = new StringBuilder();
    int spacer = 0;
    while(length > 0){
        if(spacer == spacing){
            sb.append(spacerChar);
            spacer = 0;
        }
        length--;
        spacer++;
        sb.append(randomChar());
    }
    return sb.toString();
}

The likelihood of generating two keys that are duplicates is  \$\frac{1}{36^{16}} \approx 8\cdot 10^{-24}\$. So if you generate 10000 (ten thousand) bulk keys, the likelyhood of not having any duplicates is: $$(1-\frac{1}{36^{16}})^{10000}=99.99999999999999999987\%$$.

In other words, you are not very likely to get duplicates as long as your random function is good (hence the use of SecureRandom instead of Math.random(). To be certain you have to always check against your database anyway as a last step so I would just not bother with duplicate checking on the client side as the likelihood of finding duplicates is so very low.


Generate String with Random Consonants and Vowels in C Replace all c / C / v / V in stdin with lower case, upper case consonants / vowels respectively. All other input is passed through.

Anything that can be improved? Looking at maintainability first, performance second.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>

static const char lower_consonants[] = "bcdfghjklmnpqrstvwxyz";
static const char upper_consonants[] = "BCDFGHJKLMNPQRSTVWXYZ";
static const char lower_vowels[] = "aeiou";
static const char upper_vowels[] = "AEIOU";

#define RND_CHAR(str) ((str)[rand() % (sizeof(str) - 1)])

char to_char(char c)
{
    switch (c) {
    case 'c': return RND_CHAR(lower_consonants); break;
    case 'C': return RND_CHAR(upper_consonants); break;
    case 'v': return RND_CHAR(lower_vowels); break;
    case 'V': return RND_CHAR(upper_vowels); break;
    default: return c;
    }
}

int main(void)
{
    srand(time(NULL));
    for (int c; (c = getchar()) != EOF;)
        putchar(to_char(c));
    fflush(stdout);
}

Makefile:

CFLAGS:=-std=c11 -W -Wall -pedantic -O2 -g

.PHONY: all
all: wordify

.PHONY: clean
clean::
    $(RM) wordify *.[adios] *.gcda *.gcno *.gcov

This is generally quite pleasant. Only a few observations:

Character input loop

The construct

for (int c; (c = getchar()) != EOF;)

is a little easy to misread.

I'm always very tempted to use a for loop for this kind of thing (also various kinds of pointer manipulation of a similar form) but it tends to end up being rather awkward.

while ((c = getchar()) != EOF)

Is an overwhelmingly common C idiom, and I'd be inclined to use that form instead (even if you do end up with a slightly lonely-looking variable declaration at the top).

Redundant reference data

As another answer pointed out, it seems a bit superfluous to maintain completely separate sets of reference data for upper- and lower- case letters. Using toupper in to_char would be one approach; personally, I would be tempted to wrap to_char in another function which checked the case of its argument and called toupper as appropriate---but that might feel like overkill for only a couple of character classes.

The explicit list of consonants also feels a little funny to me. I think of consonants as "those letters which are not vowels", and there enough of them that I'd have to think far too hard if I were to type them all out. I'd be certainly tempted to generate the consonant set at runtime, iterating over the range 'a' to 'z' and excluding the vowels (which I would explicitly enumerate).

Naming: to_char

The signature char to_char(char c) looks a little odd to me. Converting characters to characters? I'd probably instinctively reach for something like

char gen_char(char kind)

(for generate character), but that's open to debate.

Redundant breaks

The break statements in to_char don't hurt, but they are dead code, so they take up mental space. I would be inclined to drop them---and to align the return from the default case with the others, to emphasise that all of the cases return directly.

No return from main

I know that the C standard allows it, but it will still always look wrong to me. Depending on the context for this program, other people might look at the source and have mental space occupied by the sense that the lack of a return from main looks wrong. But that is essentially entirely a matter of opinion, and arguably I'm the one that's wrong there.

As I said though, overall it's really rather pleasant.


Python function to find the next smaller number with same set of digits Problem: Write a function that takes a positive integer and returns the next smaller positive integer containing the same digits.

My solution:

def next_smaller(n):
    import itertools
    x = list(str(n)) #list of digits
    #permutates and joins digits
    x = [int(''.join(i)) for i in list(itertools.permutations(x))]
    x.sort() #sorts numerically
    #picks number previous to original in list
    r = [x[i-1] for i in range(0,len(x)) if x[i] == n][0]
    if r > n: return -1 #returns -1 if no solution
    else: return r #returns solution

My code works, however it is terribly inefficient with very large numbers. Any way to make this code more efficient and cut down the execution time?

The solution to a good algorithm is a clear logic. Your code uses the "hammers" that are present, spring to mind, and do a tiny bit more than needed. So start to find a logic first.

To get the next smaller number the most strict algorithm would be:

Example: 1253479

Start from the last digit to the first as long as the digits are decreasing or equal. 12[5]+increasing 3479, 5 > 3.
From the increasing tail pick the element just smaller (4), and reverse the tail. 12(4)+decreasing 97[5]3.

Result: 1249753

The math is relatively simple, and the algorithm linear to the length.

  As you see the code can do with the "hammers", functions, you use.

Explanation (on request)

Say you have a number 1253479 = prefix 12, digit 5, and increasing suffix 3479. (5 > 3 hence a smaller permutation on digit+suffix is possible)

An increasing (non-decreasing) suffix 3479 is the first minimum permutation; other permutations like 4379 are greater.

A decreasing (non-increasing) suffix 9753 is the last maximum permutation; other permutations like 9735 are smaller.

Hence a permution++ resp. permutation-- works like:

1 2 5 3 4 7 9
     \ / / /
= =
     / \ \ \
1 2 4 9 7 5 3

Roughly reminiscent of increasing/decreasing an integer on paper with a suffix 00...0.

A proof would be more for a math forum.

As this answer has its probably deserved downvotes (it insists on a redesign), I leave it at that.


Calling API only once on changing segments I am working on a screen which has segmented control and has 3 segments (year value 2019, 18, 17) and what I am doing is that on each segment click I have to reload data inside a tableview (getting data from an API request).

let purchaseResponse = inputs.selectedTab.map { index -> String in
        switch index {
        case 0:
            return "2019"
        case 1:
            return "2018"
        case 2:
            return "2017"
        default:
            return "2019"
        } }
        .distinct()
        .flatMapLatest { year in
            return purchaseService.fetchPurchaseHistory(forYear: year, pageNumber: 1)
        }
        .trackActivity(inputs.indicator)
        .materialize()
        .share()

Right now this is what I have, and it works fine, as you can see I am using the Distinct operator which is like this:

extension Observable where Element: Hashable {

func distinct() -> Observable<Element> {
    var set = Set<Element>()
    return flatMap { element -> Observable<Element> in
        objc_sync_enter(self); defer {objc_sync_exit(self)}
        if set.contains(element) {
            return Observable<Element>.empty()
        } else {
            set.insert(element)
            return Observable<Element>.just(element)
        }
    }
}
}

Can I somehow refactor this, or maybe use builtin operators? The reason to use Distinct was that if the user clicks a segment, next time it shouldn't get data from the API.

The operator you are looking for is distinctUntilChanged(). It skips only equal items followed one by one.

Or distinct() operator which skips the items what has been even once emitted in the sequence. In your case the fetchPurchaseHistory method could be triggered no more than 3 times by this observable with distinct operator


Wrapper for complicated Range manipulation I'm playing around with a class module to try to wrap up some complicated(ish) Range manipulation in Excel-VBA.

I might have a situation where I know the red range spans my target (the target is to have it's contents cleared) and all cells below the red range should go. Green areas are cells used that should not be deleted:

Another example: I only know the red cell - and all used cells just to it's right and below it are to be cleared. Again all green cells should be left alone:

Usually the above is done via manipulation by finding extreme cells (rows.count...toLeft etc) or the CurrentRegion property of Range. I've attempted to wrap that functionality into a Class Module:

Class Module Name: TargetRange

Private fCurrentRange As Excel.Range

''::
''::
Public Enum expandDirection
    toTheRight = 1
    fromTheRight = 2
    downwards = 3
    fromSheetBase = 4
End Enum

''::  read/write property - initial range
''::
Public Property Let CurrentRange(ByRef rng As Excel.Range)
    Set fCurrentRange = rng
End Property
Public Property Get CurrentRange() As Excel.Range
    Set CurrentRange = fCurrentRange
End Property

':: Expand
'::   this will need to use an enumerated type as options
'::
Public Sub expand(ByVal aDir As expandDirection)

Dim lstCell As Excel.Range
Dim lstRow As Long
Dim lstCol As Long

Set lstCell = Me.CurrentRange.Cells(Me.CurrentRange.Cells.Count)
lstRow = Me.CurrentRange.Cells(Me.CurrentRange.Cells.Count).Row
lstCol = Me.CurrentRange.Cells(Me.CurrentRange.Cells.Count).Column

If (aDir = downwards) Then

    Dim newLstRow As Long
    'newLstRow = lstCell.End(Excel.xlDown).Row
    newLstRow = Me.CurrentRange.CurrentRegion.Cells(Me.CurrentRange.CurrentRegion.Cells.Count).Row

    Me.CurrentRange = _
        Range( _
            Me.CurrentRange.Cells(1), _
            Me.CurrentRange.Worksheet.Cells( _
                newLstRow, _
                lstCol _
                ) _
            )
End If

If (aDir = toTheRight) Then

    Dim newLstCol As Long
    newLstCol = lstCell.End(Excel.xlToRight).Column

    Me.CurrentRange = _
        Range( _
            Me.CurrentRange.Cells(1), _
            Me.CurrentRange.Worksheet.Cells( _
                lstRow, _
                newLstCol _
                ) _
            )
End If

End Sub

I'm using it like this:

Dim r As TargetRange
Set r = New TargetRange
r.CurrentRange = wb.Sheets("foo").Range("A2")
r.expand downwards
r.expand toTheRight
r.CurrentRange.ClearContents

Questions:

As it stands could the class module be improved?
Are there some obvious methods missing from the class that could be added?
Is the whole thing a waste of time? (am I reinventing the wheel?)

Using your method, I would opt for a Select Case when determining which way to go.

Sub expand(ByVal rangeToExpand As Range, ByVal whatDirection As String)
    Dim lastRow As Long
    Dim lastColumn As Long

    Select Case whatDirection

        whatDirection = "toTheRight"
            lastRow = rangeToExpand.CurrentRegion.Row
            lastColumn = Cells(rangeToExpand.Row, rangeToExpand.CurrentRegion.Column.Count).End(xlToRight).Column
            Set rangeToExpand = Range(rangeToExpand, Cells(lastRow, lastColumn))

        whatDirection = "fromTheRight"
            lastRow = rangeToExpand.CurrentRegion.Row
            lastColumn = Cells(rangeToExpand.Row, rangeToExpand.CurrentRegion.Column.Count).End(xlToLeft).Column
            Set rangeToExpand = Range(rangeToExpand, Cells(lastRow, lastColumn))

        whatDirection = "downwards"
            lastColumn = rangeToExpand.CurrentRegion.Column
            lastRow = Cells(rangeToExpand.CurrentRegion.Rows.Count, rangeToExpand.Column).End(xlUp).Row
            Set rangeToExpand = Range(rangeToExpand, Cells(lastRow, lastColumn))

        whatDirection = "fromSheetBase"
            lastColumn = rangeToExpand.CurrentRegion.Column
            lastRow = Cells(rangeToExpand.CurrentRegion.Rows.Count, rangeToExpand.Column).End(xlDown).Row
            Set rangeToExpand = Range(rangeToExpand, Cells(lastRow, lastColumn))
    End Select

    rangeToExpand.ClearContents
End Sub

That way you don't have to check every if every time. If my method is off for the directions, I probably misunderstood the goal of each string, but I don't see them in the original.

Then all you need to do is pass a range and a string to expand and it does all the work. Or make expand a function and pass the range back to the caller.

Also, as I mentioned in the comments, I'm not sure .CurrentRegion is the way to go. If working with A1 in here - these are the CurrentRegions -

You can double check it, I'm on 2007 and just used

Sub testing()
    Dim rng As Range
    Set rng = Range("A1")
    rng.CurrentRegion.Select
End Sub

So anything that's a connection to the cell you've chosen, will be part of the CurrentRegion - even if it's all snakey like that.

So your first picture the CurrentRegion = D5:G40 and the second picture CurrentRegion = D4:L40 assuming the red cell(s) is the selection.

As to how to fix this - I'm not sure. Maybe ask for the input to be the "header" row, but that only works if you want to clear all the way down. It defeats the whole purpose of this. I didn't mean to point out a problem and not provide a solution - sorry.


Linear algebra module I'm working on a linear algebra module to improve my knowledge with mathematics and, because I'll need a lightweight linear algebra module for my future work with Vulkan!

I tried to keep a blas-like routine style, for exemple, the related functions are called like this:

template <typename T, std::size_t M, std::size_t N>
Matrix<T, N, M> transposed(const DenseBase<T, M, N>& mat)

The whole module is templated and works with compile-time size matrices. I use an std::vector as a storage container because I want to keep it allocated on the heap (for large matrices)

The 2nd goal is to provide an access to row/column element by reference, for example:

Matrix<float, 10, 10> matrix{};
matrix.col(1)[1] = 5; // 2nd element of 2nd matrix's column is now set to 5

To achieve this, I have a hierarchy with a base class:

template <typename T, std::size_t M, std::size_t N>
class DenseBase
{
 public:
 using Matrixbase = Matrix<T, M, N>;
 using Row = Rowview<DenseBase, T, N>;
 using Col = Colview<DenseBase, T, M>;

 using ConstRow = const Rowview<const DenseBase, const T, N>;
 using ConstCol = const Colview<const DenseBase, const T, M>;

  Row row(std::size_t index) { return Row{*this, index}; }
  Col col(std::size_t index) { return Col{*this, index}; }

  ConstRow row(std::size_t index) const { return ConstRow{*this, index}; }
  ConstCol col(std::size_t index) const { return ConstCol{*this, index}; }
  T& operator()(std::size_t row, std::size_t col) { return coeff(row, col); }
  T& operator[](std::size_t index) { return coeff(index); }

  const T& operator()(std::size_t row, std::size_t col) const { return coeff(row, col); }
  const T& operator[](std::size_t index) const { return coeff(index); }

  virtual T& coeff(std::size_t row, std::size_t col) = 0;
  virtual const T& coeff(std::size_t row, std::size_t col) const = 0;

  virtual T& coeff(std::size_t index) = 0;
  virtual const T& coeff(std::size_t index) const = 0;
};

Then, I have a Rowview, Colview class which takes a Densebase as an argument and implements the coeff functions:

template <typename Parent, typename T, std::size_t N>
class Rowview : public DenseBase<T, 1, N>
{
 public:
  Rowview(Parent& parent, std::size_t row) : parent_{parent}, row_{row} {}

  T& coeff(std::size_t, std::size_t col) { return parent_(row_, col); }
  T& coeff(std::size_t index) { return parent_(row_, index); }

  const T& coeff(std::size_t, std::size_t col) const { return parent_(row_, col); }
  const T& coeff(std::size_t index) const { return parent_(row_, index); }

 private:
  Parent& parent_;
  std::size_t row_;
};

And finally, the matrix class itself, with the std::vector and the implementation of coeff methods to access its elements.

I really need some advice, maybe some ideas about how to improve the architecture. I don't like the idea of an abstract template class, but I have no alternatives yet.

Note that you can find the whole code on GitHub.

Since you are using matrices, I don't think that you need the runtime polymorphic behaviour offered by virtual but that you created virtual functions so that you could call methods of the derived class from methods of the base class. If this is the case, you could use static polymorphism instead, thanks to the Curiously Recurring Template Pattern idiom (CRTP).

In other words, make DenseBase take a Derived template parameter which corresponds to the type of the derived class:

template <typename T, std::size_t M, std::size_t N, typename Derived>
class DenseBase
{
    // ...
}

Then make the derived classes feed their own type to this template parameter:

template <typename Parent, typename T, std::size_t N>
class Rowview : public DenseBase<T, 1, N, Rowview<Parent, T, N>>
{
    // ...
}

Now, add a couple of methods to DenseBase to make a downcast when needed. Since you know the derived type at compile time thanks to the template parameter Derived, you can make a safe downcast with a static_cast:

Derived& derived() { return static_cast<Derived&>(*this); }
const Derived& derived() const { return static_cast<Derived&>(*this); }

Now you can rewrite the coeff methods in DenseBase so that they don't need to use virtual anymore to call the coeff methods from the derived class:

T& coeff(std::size_t row, std::size_t col) { return derived().coeff(row, col); }
const T& coeff(std::size_t row, std::size_t col) const { return derived().coeff(row, col); }

T& coeff(std::size_t index) { return derived().coeff(index); }
const T& coeff(std::size_t index) const { return derived().coeff(index); }

That way, you have some of the benefits of polymorphism without having to pay the cost of runtime polymorphism which I feel you won't need for such a class. That's a little bit tricky to understand and use at first, but it has clear benefits.


Slow update function I've been fiddling with Unity, trying to make nice walking/running animations. So, i found the Stealth tutorial and I used pretty much the same animator as the AI, only I only use Speed and AngularSpeed and, of course, added a control script. The controls work as intended but the whole script is laggy. I know, my Update function is really loaded, but I'm out of ideas. Any input appreciated even if it is totally irrelevant :)

using UnityEngine;
using System.Collections;

public class Movement : MonoBehaviour {

    public float speed = 0f;        //used public just to check the values

    public float angularSpeed = 0f; //same

    public float accTime = 1f;
    public float decTime = 1f;
    public float turnTime = 2.5f;

    public float speedOffset = 0f;      //offsets set to let lerp "go faster" when approaching the target value
    public float angularSpeedOffset = 0f;   //same

    private float axisX = 0f;
    private float axisY = 0f;

    private int speedHash = Animator.StringToHash ("Speed");
    private int angularSpeedHash = Animator.StringToHash ("AngularSpeed");

    private Animator anim;

    void Awake () {
        anim = GetComponent<Animator>();
    }

    void Update () {
        SpeedCalc ();
        AngularSpeedCalc ();

        anim.SetFloat (speedHash, speed);
        anim.SetFloat (angularSpeedHash, angularSpeed);
    }

    void SpeedCalc () {
        axisY = Input.GetAxis ("Vertical");

        if (axisY > 0)
            speed = Mathf.Lerp (speed, 5.7f * axisY + speedOffset, accTime * Time.deltaTime);       // targets multiplied by axis input for analogue sticks to be... analogue..
        else
            speed = Mathf.Lerp (speed, 0f - speedOffset * 0.5f , decTime * Time.deltaTime);

        if (speed >= 5.7f) speed = 5.7f
        else if (speed <= 0f) speed = 0f;
    }

    void AngularSpeedCalc () {
        axisX = Input.GetAxis ("Horizontal");

        if (axisX > 0 && angularSpeed >= 0f)
            angularSpeed = Mathf.Lerp (angularSpeed, 2.5f * axisX + angularSpeedOffset, turnTime * Time.deltaTime);
        else if (axisX < 0 && angularSpeed <= 0f)
            angularSpeed = Mathf.Lerp (angularSpeed, 2.5f * axisX - angularSpeedOffset, turnTime * Time.deltaTime);
        else {
            if (Mathf.Abs (angularSpeed) > 0.2f)
                angularSpeed = Mathf.Lerp (angularSpeed, 0f, turnTime * Time.deltaTime);
            else
                angularSpeed = 0f;
        }

        if (angularSpeed >= 2.5f) angularSpeed = 2.5f;
        else if (angularSpeed <= -2.5f) angularSpeed = -2.5f;
    }
}

You have a lot of magic numbers in your code which should be extracted to meaningful named constants.

if (Mathf.Abs (angularSpeed) > 0.2f)
if (angularSpeed >= 2.5f) angularSpeed = 2.5f;

  ...

Using braces {} for single statements if or else parts shouldn't be omitted to make your code less error prone.

Unneeded checks should be omitted too like

if (angularSpeed >= 2.5f) angularSpeed = 2.5f;
else if (angularSpeed <= -2.5f) angularSpeed = -2.5f;

should be

if (angularSpeed > 2.5f)
{
    angularSpeed = 2.5f;
}
else if (angularSpeed < -2.5f)
{
    angularSpeed = -2.5f;
}

Don't shorten your variable names because it reduces readability.  E.g

private Animator anim;

should be

private Animator animator;


Find next occurring Friday (or any dayOfWeek) Given a particular Date,  want to find the next occurring Friday.  I solved it using the below code.  Various tested scenarios work good for me.

Are there any boundary conditions the code might break?
Can it be written any better?

function dates() {
    var dayOfWeek = 5;//friday
    var date = new Date(2013, 10, 13);
    var diff = date.getDay() - dayOfWeek;
    if (diff > 0) {
        date.setDate(date.getDate() + 6);
    }
    else if (diff < 0) {
        date.setDate(date.getDate() + ((-1) * diff))
    }
    console.log(date);
}

Since you say "Given a particular Date" and "next occurring Friday (or any dayOfWeek)" I think both should be parameters to your function. As it is written, it will only return November 15 always.

function dates(date, dayOfWeek)

But then you should check that dayOfWeek is in range (0-6, thanks SridharVenkat) and probably that date is a valid Date object.

If you do this, you also have to consider not modifying the date parameter and copy it instead.

The names on the variables are clear enough, but not of the function, dates is too broad. getNextDayOfWeek says a lot about what this function does.

function getNextDayOfWeek(date, dayOfWeek)

The logic is correct, I think, I only tested it with a few numbers. But you instead of + ((-1) * diff)) you could simply do - diff.

Finally, you're always outputting the result to the console. If this is more than an exercise, you should return the result, and let the user of the function do with it whatever he/she needs.

Edit: As SridharVenkat said, the range should be 0-6 not 1-7.

Also, using modulo like AlexAtNet suggested you can reduce the code with
resultDate.setDate(date.getDate() + (7 + dayOfWeek - date.getDay()) % 7)

My (edited) proposal:

function getNextDayOfWeek(date, dayOfWeek) {
    // Code to check that date and dayOfWeek are valid left as an exercise ;)

    var resultDate = new Date(date.getTime());

    resultDate.setDate(date.getDate() + (7 + dayOfWeek - date.getDay()) % 7);

    return resultDate;
}

PS: Please correct me if I said something wrong of if there's more to improve.


Ruby on rails with active record perfomance I've got relation in my database as following :

Customer has many Orders
Order has many Parts
Order has many Upc

So my requirement is to copy all orders from one customer and attach or copy those orders (with their dependencies including parts and upcs) to another customer. With originals customers orders to be left in-tacked, so I'd basically clone orders and dependencies from customer a to customer b

Here is what I got so far, but its kinda slow (slowest first):

I'd iterate trough customers orders first and then I'd call copy_order1 method to copy order and dependencies to another customer. here is copy_order1 (which is in class Customer):

def copy_order1(source_order)
        order_attributes = source_order.attributes.merge({:customer_id => self.customer_id})
        ['id', 'deleted_at'].each{|k| order_attributes.delete k}
        ord = Order.create(order_attributes)

        source_order.parts.each do |part|
            part_attributes = part.attributes.merge({:order_id => ord.id})
            ['id', 'deleted_at'].each{|k| part_attributes.delete k}

            Part.create(part_attributes)

        end

        source_order.upcs.each do |upc|
            upc_attributes = upc.attributes.merge({:order_id => ord.id})
            ['id', 'deleted_at'].each{|k| upc_attributes.delete k}

            Upc.create(upc_attributes)

        end

        return ord
      end

And there I made slightly faster method called copy_order2 :

def copy2(destination_customer)

    ActiveRecord::Base.transaction do
      Order.includes(:customer, :part, :upc).where(:customer_id => self.id).find_in_batches(:batch_size => 50) do |batch|
        batch.each do |order|
          new_order = order.dup
          new_order.parts << order.parts.each {|part| tmp = part.dup; tmp.id = tmp.order_id = nil; tmp}
          new_order.upcs    << order.upcs.each    {|upcs| tmp = upc.dup; tmp.id = tmp.order_id = nil; tmp}

          destination_customer.orders << new_ordert
        end

        # Save one batch
        destination_customer.save!
      end
    end
  end

Can anyone suggest performance friendly way to do this from your own experience? How would you approach this issue if you had one?

The #dups are probably unnecessary. If you unset the ID ActiveRecord already "thinks" that the object is new, and #save will create a new one in the DB. But the main bottleneck is probably the amount of DB queries generated. The easiest way to gain some performance in such cases is to wrap the whole block into a transaction. The best way would probably use bulk inserts (e.g. with a gem like https://github.com/zdennis/activerecord-import).

But if you've to do so many deep copies you also should rethink if you're using the right approach (or DB) here.


Determining if all strings are parsable to int I have to loop through a list of strings and validate that all are parsable to int and not less than 1.  Also, if a string is not parsable but is null or whitespace, then it is OK and my validator should not fail.

Here's the code I have come up with which works fine but leads to method complexity:

public static bool ArePerPersonBILimitsInvalid(IEnumerable<string> bilimits)
        {
            return bilimits.Any(c =>
            {
                int d;
                if (!int.TryParse(c, out d))
                {
                    if (string.IsNullOrWhiteSpace(c))
                    {
                        return false;
                    }

                       return true;
                }

                if (d < 1)
                {
                    return true;
                }

                return false;
            });
        }

I find that negatives in method names lead to confusing code. For instance,

if (!ArePerPersonBILimitsInvalid(...))

  "Are there no BI limits that are invalid?"

becomes much more readable when we write it this way:

if (ArePerPersonBILimitsValid(...))

  "Are all BI limits valid?"

So let's fill in the blanks.

public static bool ArePerPersonBILimitsValid(IEnumerable<string> biLimits)
{
    return biLimits.All(IsBILimitValid);
}

Great, so now we need to write IsBILimitValid.

public static bool IsBILimitValid(string biLimit)
{
    if (string.IsNullOrWhiteSpace(biLimit))
    {
        return true;
    }

    int value;
    return int.TryParse(biLimit, out value) && value >= 1;
}

This way the code much more closely reflects your problem statement:

  loop through a list of strings and validate that all are parsable to
  int and not less than 1


Program to check whether two strings are anagrams of each other I have written this code and I am wondering if the code could be optimised.Please review.

 public class Anagram1{
   public static boolean check(char[] c1,char[] c2,int l1,int l2){

     /*if the lengths of the strings are not equal,they cannot be anagrams of each other */

    if(l1!=l2){
        return false;
    }

    int count = 0;

    //nested for loops to compare the characters of both strings.
    //if the characters match,then the strings are anagrams of each other.
    for(int i=0; i<l1; i++){
        for(int j=0; j<l2; j++){
            if(c1[i]==c2[j]){
                count++;
                break;      //"break" used so as to count only a single occurrence of a
            }               //character.
        }
    }
    if(count==l1 && count==l2){
        return true;
    }
    return false;
}

public static void main(String[] args) {

    String s1 = "abcd";

    String s2 = "acdb";

    char[] c1 = s1.toCharArray();

    char[] c2 = s2.toCharArray();

    int l1 = s1.length();

    int l2 = s2.length();

    System.out.println(check(c1,c2,l1,l2));
  }
}

About the algorithm (ITS NOT CORRECT). I will still review it for you:

Passing l1 and l2 in boolean check(char[] c1,char[] c2,int l1,int l2) as argument is redundant. You can use the inherent c1.length and c2.length provided by Java.
if(count==l1 && count==l2) Any one check would do as you overruled l1!=l2 very early. Remember?
Failure cases:
"aab" and "abb" both would be called anagrams. Hope you discover that.

About the algorithm:

There are simpler better ways to check if two strings are anagram. How? Sort the strings and compare character by character.
Not impressed? Do a character count using a Map for each character and compare it for other array. And there are many more. Happy Googling.


LeetCode 483: Smallest Good Base I'm posting my code for a LeetCode problem. If you'd like to review, please do so. Thank you for your time!
Problem

For an integer n, we call k>=2 a good base of n, if all digits of n
base k are 1.
Now given a string representing n, you should return the smallest good
base of n in string format.
Example 1:

Input: "13"
Output: "3"
Explanation: 13 base 3 is 111.

Example 2:

Input: "4681"
Output: "8"
Explanation: 4681 base 8 is 11111.

Example 3:

Input: "1000000000000000000"
Output: "999999999999999999"
Explanation: 1000000000000000000 base 999999999999999999 is 11.

Note:

The range of n is [3, 10^18].
The string representing n is always valid and will not have leading zeros.

Inputs
"1000000000000000000"
"999999999999999999"
"141038407950127511"
"836507047502348570"
"123489798271512411"
"995437985793784539"
"4681"
"4800"
"48000"
"480000"
"5120000"
"51200000"

Outputs

"999999999999999999"
"999999999999999998"
"141038407950127510"
"836507047502348569"
"123489798271512410"
"995437985793784538"
"8"
"4799"
"47999"
"479999"
"5119999"
"51199999"

Code
#include <cstdint>
#include <string>
#include <algorithm>

struct Solution {
    std::string smallestGoodBase(const std::string n) {
        std::uint_fast64_t num = (std::uint_fast64_t) std::stoull(n);
        std::uint_fast64_t x = 1;

        for (int bit = 62; bit > 0; --bit) {
            if ((x << bit) < num) {
                std::uint_fast64_t curr = binarySearch(num, bit);

                if (curr) {
                    return std::to_string(curr);
                }
            }
        }

        return std::to_string(num - 1);
    }

private:

    static std::uint_fast64_t binarySearch(
        const std::uint_fast64_t num,
        const
        std::uint_fast8_t bit
    ) {
        const long double dnum = (long double) num;
        std::uint_fast64_t lo = 1;
        std::uint_fast64_t hi = (std::uint_fast64_t) (std::pow(dnum, 1.0 / bit) + 1);

        while (lo < hi) {
            std::uint_fast64_t mid = lo + ((hi - lo) >> 1);
            std::uint_fast64_t sum = 1;
            std::uint_fast64_t curr = 1;

            for (std::uint_fast8_t iter = 1; iter <= bit; ++iter) {
                curr *= mid;
                sum += curr;
            }

            if (sum == num) {
                return mid;

            } else if (sum > num) {
                hi = mid;

            } else {
                lo = mid + 1;
            }
        }

        return 0;
    }
};

References

Problem

Discuss

Solution

Don't use std::uint_fast*_t
The performance gain of these types is minimal. If you really need to squeeze out the last bits of performance, it might help, but only if:

These types are actually different from the regular std::uint*_t
Implicit casts and integer promotions don't change the actual type used

The drawback is that the type names get very verbose. Also, can a std::uint_fast64_t actually hold an unsigned long long? The latter might very well be 128 bits long.
I suggest you stick with (unsigned) int when dealing with numbers that don't grow large (for example, for counting bits), size_t when you need to support arbitratily large counts, sizes and array indices, and other types as appropriate, such as unsigned long long to store the results of std::stoull().
Also use a using declaration to declare the type used to store the numbers in one place, so if you ever decide to change it, you don't have to search and replace all over the code. You can combine it with decltype() to get the type returned by an arbitrary function, like so:
class Solution {
    using value_type = decltype(std::stoull(""));
    ...
};

Avoid unnecessary one-use temporaries
There are several cases where you define a temporary variable that is only used once, and which could be avoided. The first is x, which is only used to create a constant 1 of the right type. You can do that instead like so:
if ((value_type{1} << bit) < num) {
    ...
}

The second one will still technically create a temporary, but we can use C++17's if with an init statement:
if (auto curr = binarySearch(num, bit); curr) {
    return curr;
}

When passing an integer type to std::pow(), it will convert it to double for you, so you don't have to explcitly create a temporary constant dnum, and can just write:
value_type hi = std::pow(num, 1.0 / bit) + 1;

Note that you also don't need to explicitly cast it back to an integer here.


LeetCode 1615: Maximal Network Rank I'm posting a solution for LeetCode's "Maximal Network Rank". If you'd like to review, please do so. Thank you!
Problem
There is an infrastructure of n cities with some number of roads connecting these cities. Each roads[i] = [ai, bi] indicates that there is a bidirectional road between cities ai and bi.
The network rank of two different cities is defined as the total number of directly connected roads to either city. If a road is directly connected to both cities, it is only counted once.
The maximal network rank of the infrastructure is the maximum network rank of all pairs of different cities.
Given the integer n and the array roads, return the maximal network rank of the entire infrastructure.
class Solution:
    def maximalNetworkRank(self, n: int, roads: List[List[int]]) -> int:
        indegrees = collections.defaultdict(list)
        for i in range(n):
            for a, b in roads:
                if i == a:
                    indegrees[i].append(f'{i}-{b}')
                    indegrees[i].append(f'{b}-{i}')
                if i == b:
                    indegrees[i].append(f'{i}-{a}')
                    indegrees[i].append(f'{a}-{i}')

        max_net = 0
        for i in range(n):
            for j in range(n):
                if f'{i}-{j}' or f'{j}-{i}' in indegrees[i]:
                    net = len(set(indegrees[i] + indegrees[j])) // 2
                    max_net = max(max_net, net)

        return max_net

Avoid converting data structures to string unnecessarily
Just store tuples of values in indegrees instead of strings:
if i == a:
    indegrees[i].append((i, b))
    indegrees[i].append((b, i))

Iterate over the keys in indegrees directly
In this piece of code:
for i in range(n):
    for j in range(n):
        if f'{i}-{j}' or f'{j}-{i}' in indegrees[i]:
            ...

What you are basically saying is: for every possible pair of integers, check if it's in indegrees, and if so, do something with it. That's very inefficient if there only few elements in indegrees. Why not just iterate over the keys of indegrees instead? You can do it with strings, but then you have to parse each key back into two values, if you store tuples instead it becomes really simple:
for i, j in indegrees:
    ...


JavaScript functions and global variables I have a getValue function which just grabs some numbers from an HTML page.

How can I access those variables in later functions without passing them down the entire chain of functions? As they sit within a function they are not global?

When I call the addition function, it will need to access the variables in the getValue function.

In Java, I would have used a separate class with a normal getter and setter pattern but unsure how to replicate something similar.

I feel like I am missing a key JavaScript concept here.

    function getValue() {
    var decimal = parseInt(parent.document.getElementById('CF_9813416').value);
    var a = parseFloat(parent.document.getElementById('CF_9814331').value);
    var b = parseFloat(parent.document.getElementById('CF_9814407').value);
    var c = parseFloat(parent.document.getElementById('CF_9814615').value);
    }

    function radioButtons() {

        for (var i = 0; i < parent.document.Form1.CF_9813571.length; i++) {
            if (parent.document.Form1.CF_9813571[i].checked) {
                var sign = parent.document.Form1.CF_9813571[i].value;
            }
        }
        radioResult(sign);
    }

    function radioResult(sign) {

        if (sign == '9813524') {
            addition();
        } else if (sign == '9813527') {
            subtraction();
        } else if (sign == '9813530') {
            multiplication();
        } else devision();
    }

    function addition() {

        result = a + b + c;
        writeValue(result, decimal);
    }

    function subtraction() {

        result = a - b - c;
        writeValue(result, decimal);
    }

    function multiplication() {

        result = a * b * c;
        writeValue(result, decimal);
    }

    function devision() {

        result = a / b / c;
        writeValue(result, decimal);
    }

    function writeValue(result, decimal) {
        parent.document.getElementById('CF_9814616').value = result.toFixed(0);
        parent.document.getElementById('CF_9814617').value = result.toFixed(decimal);
    }
</script>

Yes, you are right, decimal, a, b and c are not accessible by other functions because they are local to getValue(). To make them global, you need to declare them outside of your function, like this:

Solution #1

var decimal;
var a;
var b;
var c;

function getValue(){
 decimal = // assign decimal
 a = // assign a
 // assign rest of the variables
}

function useVariables(){
   return a + b + c;
}

Using global variables, however, is considered a bad practice so I would move onto another solution.

Solution #2

Have 'getter functions' for each of your variables. Instead of having getValue(), have getDecimal(), getA(), getB() and getC() functions (you might consider giving these variables more meaningful names). Your code would turn into something like this:

function getA(){
  return parseFloat(parent.document.getElementById('CF_9814331').value);
}

// functions for getB() and getC()

function useVariables(){
  return getA() + getB() + getC();
}

This is a more acceptable solution because you delay getting the variables until you need them and you also do not have global variables. If the values change you get the updated values as well without having to call getValue() again to 'refresh' your data.


Basic Markov Chain Algorithm So I have a file(story.dat) with a randomly generated story like:
"Sleeping in his car was never the plan but sometimes things don't work out as planned. This..."
It will print out each word and the corresponding word to it like so:
0."It: was,
1."Pepsi-Cola.": He,
2."This: is,
3./: 50.,
4.50: /,
5.50.: I,
6.A: long,
7.All: that,
8.He: didn't,knew,had,
9.I: think,had,
10.I'd: love,
11.I'm: certain,
12.It: was,
13.Or: at,
14.She: knew,
15.Sleeping: in, ...

And then it prints the story like so:

have a cup of everything. The inside was ten, and a little strange,
but he was either exhausted of days ago. It was supposed to hear from
you today and fondling over it. He knew what made the last option is
to make a year ago. It was supposed to make this case, was in months.
The third is the sun. He knew...

Here's the code:
// #include files
namespace Randomiser
{
    std::mt19937 generator{ static_cast<std::mt19937::result_type>(std::time(nullptr)) };
}
int getRandomNumber(int min, int max)
{
    std::uniform_int_distribution<int> rand{ min, max };
    return rand(Randomiser::generator);
}

void markovIt(const std::map<std::string, std::vector<std::string>>& m)
{
    std::string temp{};
    std::vector<std::string> possibilities{};
    std::string next{};

    int i{ 0 };
    int randomNumber{ getRandomNumber(0, m.size()) };
    for (const auto& n : m)
    {
        if (i == randomNumber)
        {
            temp = n.first;
        }
        ++i;
    }

    auto found{ m.find(temp) };
    i = 0;
    while(found != m.end())
    {
        if (i == 255)
            break;

        possibilities = found->second;
        next = possibilities.at(getRandomNumber(0, possibilities.size() - 1));

        std::cout << temp << " ";
        temp = next;

        found = m.find(temp);
        ++i;
    }
}

int main()
{
    std::ifstream wordStream{ "story.dat" };
    std::string fWord;
    std::string sWord;
    std::vector<std::string> temp;
    std::map<std::string, std::vector<std::string>> gram{};
    int i{ 0 };
    while(wordStream)
    {
        wordStream >> fWord;
        if (i != 0)
            gram[sWord].push_back(fWord);
        wordStream >> sWord;
        gram[fWord].push_back(sWord);
        if (i == 0)
            i++;
    }

    i =  0;
    for (const auto& n : gram)
    {
        std::cout << i << "." << n.first << ": ";
        for (const auto& key : n.second)
            std::cout << key << ",";
        std::cout << '\n';
        ++i;
    }

    markovIt(gram);
    std::cout << '\n';

    return 0;
}

The implementation feels awful and I believe it could be optimised alot further. I'm just not sure where to take it.

Use std::random_device instead of time()
Stay within C++, and use a real random device to seed the generator, like so:
std::random_device rd;
std::mt19937 generator{rd()};

Consider turning Randomiser into an anonymous namespace
Does this namespace need a name? If only getRandomNumber() uses Randomiser::generator, and it's in the same source file, you can make it an anonynous namespace instead, just omit "Randomiser".
Be consistent with types
Avoid mixing types like int and size_t. It might seem to work fine, but it is easy for errors to happen this way. Before changing types, consider that sometimes you don't really need to specify a type explicitly, and can instead use template<> or auto. For example:
template<typename T>
auto getRandomNumber(T min, T max)
{
    std::uniform_int_distribution<T> rand{ min, max };
    return rand(Randomiser::generator);
}

However, you will now get some compiler errors, because you were actually calling getRandomNumber() with min and max having different types, like in:
int randomNumber{ getRandomNumber(0, m.size()) };

This is actually a good thing, the compiler is telling you something fishy is going on. In fact, m.size() returns a size_t, which can be bigger than what an int can hold on most 64-bit platforms, as well as being unsigned instead of signed. Considering that min is always 0 in your code, I would instead write:
template<typename T>
auto getRandomNumber(T max) {
    std::uniform_int_distribution<T> rand{ 0, max };
    return rand(Randomiser::generator);
}

...

auto randomNumber = getRandomNumber(m.size());

Note that you probably meant getRandomNumber(m.size() - 1) here. This brings me to:
Consider writing a getRandomElement() function
You actually use the random numbers to pick random elements out of containers. So write a function to do exactly that:
template<typename T>
static auto &getRandomElement(const T &container)
{
    std::uniform_int_distribution<size_t> rand{ 0, container.size() - 1 };
    return *std::next(std::begin(container), rand(Randomiser::generator));
}

Simplifying markovIt()
Now that we have the above, we can simplify markovIt() greatly. It can be rewritten as:
void markovIt(const std::map<std::string, std::vector<std::string>>& m)
{
    // Get the initial word
    auto word = getRandomElement(m).first;

    for (int i = 0; i < 255; ++i)
    {
        std::cout << word << " ";

        // Find the entry in m that matches word
        auto entry = m.find(word);
        if (entry == m.end())
            break;

        // Select a random word from the list of possibilities
        word = getRandomElement(entry->second);
    }
}

Move special cases out of loops if possible
While loading story.dat, you use the variable i do determine if you need to push back the first word. You can move that special case outside of the loop. The compiler will probably do this for you even if you don't, but it also cleans up the code if you do so in this case:
wordStream >> fword;
wordStream >> sWord;
gram[fWord].push_back[sWord];

while(wordStream)
{
    wordStream >> fWord;
    gram[sWord].push_back(fWord);
    wordStream >> sWord;
    gram[fWord].push_back(sWord);
}

However, why read two words in each loop iteration? You can rewrite this to:
wordStream >> fWord;

while(wordStream)
{
    wordStream >> sWord;
    gram[fWord].push_back(sWord);
    fWord = sWord;
}

Move more functionality into functions
You are doing too much inside main(). Move more functionality into functions, and keep main() as high-level as possible. For example:
static auto load_gram(const std::string &filename)
{
    std::map<std::string, std::vector<std::string>> gram{};
    std::ifstream wordStream{ filename };

    // load the data into gram here
    ...

    return gram;
}

int main(int argc, char *argv[])
{
    auto gram = load_gram("story.dat");
    markovIt(gram);
}


Ensuring that events raised by system.diagnostics process class happen in the parent thread I'm quite new to threading primitives in C# and was hoping you might be able to suggest improvements to this. I need to ensure that the XXX call below happens within the calling thread (XXX is a foreign call into a thread-unsafe library), so I used a queue here. It seems a bit like there should be a better primitive for this. Maybe delegates are applicable somehow? I don't understand delegates.

I also have to wonder if I've gotten this whole scheme right in the first place! Maybe there's a deadlock I'm not seeing. Threading is so tricky.

As an additional restriction, it's very important that this works on .NET 3.5.

    public void RunProc(AutoResetEvent killSubProc)
    {
        using (Process process = new Process())
        {
            var timeout = 8000;
            var channel = new Queue<string> {};

            process.StartInfo.FileName = "blah.exe";
            process.StartInfo.Arguments = @"stuff";

            process.StartInfo.UseShellExecute = false;
            process.StartInfo.RedirectStandardOutput = true;
            process.StartInfo.RedirectStandardError = true;
            process.EnableRaisingEvents = true;

            using (AutoResetEvent channelWaitHandle = new AutoResetEvent(false))
            {
                process.OutputDataReceived += (sender, e) => {
                    if (e.Data != null)
                    {
                        lock (channel) { channel.Enqueue("STDOUT"); channel.Enqueue(e.Data); }
                        channelWaitHandle.Set();
                    }
                };
                process.ErrorDataReceived += (sender, e) =>
                {
                    if (e.Data != null)
                    {
                        lock (channel) { channel.Enqueue("STDERR"); channel.Enqueue(e.Data); }
                        channelWaitHandle.Set();
                    }
                };
                process.Exited += (sender, e) =>
                {
                    lock (channel) { channel.Enqueue("EXIT"); }
                    channelWaitHandle.Set();
                };

                process.Start();

                process.BeginOutputReadLine();
                process.BeginErrorReadLine();

                bool running = true;
                while (running)
                {
                    int idx = WaitHandle.WaitAny(new WaitHandle[] {killSubProc, channelWaitHandle});
                    if (idx == 0)
                    {
                        process.Kill();
                        running = false;
                    }
                    else
                    {
                        lock (channel)
                        {
                            while (channel.Count > 0)
                            {
                                var item = channel.Dequeue();
                                XXX(item);

                                if (item == "EXIT")
                                {
                                    running = false;
                                }
                            }
                        }
                    }
                }
            }
        }
    }

If you want a pretty solution, you would have to separate your data source (process in your case) from data processor (your while loop), hide implementations behind interfaces and then wire those interfaces together via events or aggregation. This is actually a pretty common problem called "Producer-Consumer", you should be able to find multiple decent implementations on the net.

If you want a simple solution though, yours is probably fine. A few things you might want to improve:

Reduce nesting in your while loop to avoid arrow code. This can be done by using break/continue/return statements or by extracting logic to separate method or entity.
Depending on how often you receive messages and on how much time XXXtakes, you might want to release lock before calling XXX.
You might want to wait for process to exit before returning form your method. This will ensure, that everything went smoothly, and there is no process left hanging somewhere due to some error in your code.
If XXX signature is not set in stone, you should probably replace strings with complex object:

class Message
{
   //use enum instead of hardcoded strings, such as "STDERR"
   public MessageType Type { get; set; }
   //actual data from process
   public string Data { get; set; }
}

You should remove hardcoded "magic" strings either way.