Making an image slideshow I wrote a function to make a image slideshow in javascript using for() loop, setTimeout() and setInterval(), but I have doubts that is this a right approach to use a for() loop and setTimeout() together?

Is there a better way to do it...?

var myImage = document.querySelectorAll(".banner");
var arrayImage = ["https://via.placeholder.com/150x150/f00/fff?text=Slider1", "https://via.placeholder.com/150x150/f0f/fff?text=Slider2", "https://via.placeholder.com/150x150/00f/fff?text=Slider3"];
var imageIndex = 0;

function slideChange() {
  for (var i = 0; i < arrayImage.length; i++) {
    (function(i) {
      setTimeout(function() {
        myImage[i].setAttribute("src", "");
      }, 2000 * (i + 1));
      setTimeout(function() {
        myImage[i].setAttribute("src", arrayImage[i]);
      }, 2000 * i);
    })(i)
  }
}
slideChange();
setInterval(function() {
  slideChange();
}, (2000 * arrayImage.length))
.banner {
  display: block;
}

There's really no purpose in having 3 banner images if you're going to add and remove the image source from each one. You might as well just have a single image and change the source of it on each iteration.

When you do that trick with setTimeout, where your timeout is the index of the array multiplied by an offset, you are effectively creating a new thread for every iteration, which can be memory intensive when you have a lot of images. A better approach would be to use recursion.

Consider this slowLoop function.

/**
 * Execute the loopBody function once for each item in the items array,
 * waiting for the done function (which is passed into the loopBody function)
 * to be called before proceeding to the next item in the array.
 * @param {Array} items - The array of items to iterate through
 * @param {Function} loopBody - A function to execute on each item in the array.
 *      This function is passed 3 arguments -
 *          1. The item in the current iteration,
 *          2. The index of the item in the array,
 *          3. A function to be called when the iteration may continue.
 * @returns {Promise} - A promise that is resolved when all the items in the
 *      in the array have been iterated through.
 */
function slowLoop(items, loopBody) {
    return new Promise(f => {
        let done = arguments[2] || f;
        let idx = arguments[3] || 0;
        let cb = items[idx + 1] ? () => slowLoop(items, loopBody, done, idx + 1) : done;
        loopBody(items[idx], idx, cb);
    });
}

Using this you're able to keep the entire script on a single thread.

var myImage = document.querySelector("#banner");

var arrayImage = [
  "https://via.placeholder.com/150x150/f00/fff?text=Slider1",
  "https://via.placeholder.com/150x150/f0f/fff?text=Slider2",
  "https://via.placeholder.com/150x150/00f/fff?text=Slider3"
];

function slowLoop(items, loopBody) {
    return new Promise(f => {
        let done = arguments[2] || f;
        let idx = arguments[3] || 0;
        let cb = items[idx + 1] ? () => slowLoop(items, loopBody, done, idx + 1) : done;
        loopBody(items[idx], idx, cb);
    });
}

function slideChange(){
  slowLoop(arrayImage, function(img, idx, done){
    myImage.setAttribute("src", img);
    setTimeout(done, 2000);
  }).then(slideChange);
}

slideChange();
.banner {
  display: block;
}
<img id="banner">


Caesar Cypher Implementation I am learning C# and for an experiment I tried implementing the Caesar Cypher cryptography method which just offsets every character of the string n characters. How can I make this simple program better?

string original = Console.ReadLine();
string encrypted = "";
int key = 1;
foreach (var l in original)
{
    if (!l.Equals(' '))
    {
        if (l.Equals('z'))
        {
            char next = 'a';
            encrypted += next;
        }
        else
        {

            double next = (int)l + key;
            encrypted += (char)next;
        }
    }
    else
    {
        encrypted += ' ';
    }
}
Console.WriteLine(encrypted);

Afew things I noticed:

Concatenating strings using the plus operator(+), is very inefficient.  Each concatenation creates a new string.  This due to strings being immutable.  Using a char array or the StringBuilder is much more efficient.

The variable next is a double but the value you're assigning to it is 2 ints added together.  It would make much more sense to have this as a char.  In fact you should declare this before the if block to be able to use it in each condition.

Equals isn't doing what you appear to think it does.  Equals is for testing whether 2 objects are the same.  == is for testing whether 2 objects values are the same.

You appear to expect only lower case characters and spaces.  However you don't seem to have any mechanism for testing whether the string complies with that.

You can simplify getting the replacement character by using the modulus(%) operator. Something like this should work:  newChar = (char)(((oldChar - 'a') + key) % 26) + 'a'

Your code won't work right if the string has words from a language that uses two bytes to represent one character.

Right now you've hardcoded the key.  It would be better to have a method that takes the string and the key as parameters.


Saving a submitted Mongoose model, possibly with an uploaded image I have some code which saves a mongoose model from the submitted form data. However the user might also upload an image along with the form, which needs to be saved on the server (moved from the buffer).

Here is my first attempt:

// Save a new job to the database.
exports.save = function(req, res, next) {
  // If the request contains a file, generate a file name for it.
  if (req.file) req.body.logoImage = generateFileName(req.file.mimetype)

  // Save the job to the database.
  Job.create(req.body)
    .then(job => {
      // Move the file from the buffer to the uploads folder.
      if (job.logoImage) {
        moveFile(req.file.buffer, job.logoImage).then(() => {
          res.json({ status: 'Uploaded job successfully.' })
        })
      } else {
        res.json({ status: 'Uploaded job successfully.' })
      }
    })
    .catch(error => next(error))
}

I would like to avoid having to write duplicate code for sending the response.

The problem is that moveFile return a Promise so simply doing this won't work, because the response will be sent before the file is moved (and moving it might fail)!

if (job.logoImage) {
    moveFile(req.file.buffer, job.logoImage)
}
res.json({ status: 'Uploaded job successfully.' })

To get around this, I came up with the following code:

// Save a new job to the database.
exports.save = function(req, res, next) {
  // If the request contains a file, generate a file name for it.
  if (req.file) req.body.logoImage = generateFileName(req.file.mimetype)

  // Save the job to the database.
  Job.create(req.body)
    .then(job => {
      // Move the file from the buffer to the uploads folder.
      if (job.logoImage) {
        return moveFile(req.file.buffer, job.logoImage)
      }
      return Promise.resolve()
    })
    .then(() => {
      res.json({ status: 'Uploaded job successfully.' })
    })
    .catch(error => next(error))
}

Would there be a better way maybe?

Your modified code is better, but you can simplify this method even more. If you don't return anything from a function in a promise, it will be resolved with no value. There's no need to do return Promise.resolve().

.catch(error => next(error)) is equivalent to .catch(next).

Code Tells You How, Comments Tell You Why. Don't clutter your code with comments that tell you nothing more than the code they are describing. "If the request contains a file, generate a file name for it." tells me nothing more than if (req.file) req.body.logoImage = generateFileName(req.file.mimetype)

Depending on your framework, async functions might be able to simplify this logic. If you are using Express, this won't apply. However, if you are using a framework which lets you return a promise in middleware, async functions are the way to go. If next(error) is equivalent to returning a rejected promise, you can just do this:

// Save a new job to the database.
exports.save = async function(req, res) {
  if (req.file) {
    req.body.logoImage = generateFileName(req.file.mimetype)
  }

  const job = await Job.create(req.body)
  if (job.logoImage) {
    await moveFile(req.file.buffer, job.logoImage)
  }

  res.json({ status: 'Uploaded job successfully.' })
}

If Job.create doesn't use req.body.logoImage and generateFileName never returns a falsy value, you can further simplify this logic by combining the if statements.

// Save a new job to the database.
exports.save = async function(req, res) {
  const job = await Job.create(req.body)
  if (req.file) {
    await moveFile(req.file.buffer, generateFileName(req.file.mimetype))
  }

  res.json({ status: 'Uploaded job successfully.' })
}

If async functions won't work for your use case (I wouldn't want to make the exported function return a promise if it is left dangling), I would go with this logic (assuming generateFileName can return a falsy value):

// Save a new job to the database.
exports.save = function(req, res, next) {
  if (req.file) req.body.logoImage = generateFileName(req.file.mimetype)

  Job.create(req.body)
    .then(job => {
      if (job.logoImage) {
        return moveFile(req.file.buffer, job.logoImage)
      }
    })
    .then(() => {
      res.json({ status: 'Uploaded job successfully.' })
    })
    .catch(next)
}


Merging nested tables into a single table I made a function that merges nested tables into a single table.

The following function converts the headers. It takes an old table that has nested tables in its rows, takes all the headers of these and creates just one big table with one header.

This is only one part of all the creation for the new table. I basically do the same for the body and the footer.

I was wondering if there is anything I can do to improve my code.

function setTableHeader(oldTableName, newTableName) {
    $('table#'+newTableName).html($('
'));
    var firstRow = $('
'+newTableName+'
');
    var countryColumn = $('Country	');
    var row = $('

');
    var channels = $('

');
    var className = '';

    $('#'+oldTableName+' .budget-inner-table').each(function() {
        var oldInnerTable = $(this);
        var headerText = oldInnerTable.find('h3').eq(0).text();
        className  = ((insideText.indexOf('-') > 0) ? /- (.*)/.exec(insideText)[1]: insideText.replace(/ /g, '-')).toLowerCase();
        className = className.replace(' ', '-');
        var column = $('	').text(insideText).attr('id', className);
        row.append(column);
        // loops through the table row headers
        oldInnerTable.find('.budget-channel-table thead th').each(function() {
            var channelTableHeaders = $(this);
            if (channelTableHeaders.hasClass('month') || channelTableHeaders.hasClass('countryCode')) {
                return;
            }
            else {
                var insideText = channelTableHeaders.text();
            }
            var columnData = $('	').text(headerText)
                .addClass(className)
                .addClass(headerText.replace(/ /g, '-').toLowerCase().replace('.', ''));

            channels.append(columnData);
        });
    });
    row.prepend(countryColumn);
    // appends all rows to the header
    $('table#'+newTableName+' thead').append(firstRow).append(row).append(channels);
}

Suspicious code

The variable insideText seems to be used before it's defined.
Is that a global variable? Avoid global variables as much as possible.

This else block seems pointless:

else {
    var insideText = channelTableHeaders.text();
}

Something's suspicious here:

$('table#' + newTableName).html($('
'));

Note that ids must be unique in the document. So the table prefix there is redundant.

Readability

For better readability, put spaces around operators, for example instead of this;

var firstRow = $('
'+newTableName+'
');
$('#'+oldTableName+' .budget-inner-table').each(function() {

Get into the habit to write like this:

var firstRow = $('
' + newTableName + '
');
$('#' + oldTableName + ' .budget-inner-table').each(function() {

The ternary operator can be very useful sometimes.
But this expression is too complex and hard to read:

className = ((insideText.indexOf('-') > 0) ? /- (.*)/.exec(insideText)[1]: insideText.replace(/ /g, '-')).toLowerCase();

It would be better to expand this to its equivalent using if-else:

if (insideText.indexOf('-') > 0) {
    className = /- (.*)/.exec(insideText)[1];
} else {
    className = insideText.replace(/ /g, '-').toLowerCase();
}

Helper methods

You are using this kind of code in at least two places:

... = insideText.replace(/ /g, '-').toLowerCase();
... = headerText.replace(/ /g, '-').toLowerCase()

It would be good to introduce a helper function for this, for example:

function slugify(text) {
    return text.replace(/ /g, '-').toLowerCase();
}

... = slugify(insideText);
... = slugify(headerText);

Misc

Code like this can be simplified:

if (condition) {
    return;
} else {
    // do something
}

Like this:

if (condition) {
    return;
}

// do something


Python Binary Search Tree Implementation #BST
class Node:
    def __init__(self, val, left = None, right = None):
        self.val = val
        self.left = left
        self.right = right

def search(root, k):
    if root is None or root.val == k:
        return root
    elif root.val < k:
        return search(root.right, k)
    else:
        return search(root.left, k)

def minimum(root):
    if root is None or root.left is None:
        return root
    return minimum(root.left)

def maximum(root):
    if root is None or root.right is None:
        return root
    return maximum(root.right)

def successor(root, node):
    if node.right is not None:
        return minimum(node.right)
    succ = None
    while root is not None and root.val != node.val:
        if root.val > node.val:
            succ = root
            root = root.left
        else:
            root = root.right
    return succ

def predecessor(root, node):
    if node.left is not None:
        return maximum(node.left)
    else:
        pred = None
        while root is not None and root.val != node.val:
            if root.val > node.val:
                root = root.left
            else:
                pred = root
                root = root.right
        return pred

def insert(root, key):
    if root is None:
        return Node(key)
    elif root.val < key:
        return Node(root.val, root.left, insert(root.right, key))
    else:
        return Node(root.val, insert(root.left, key), root.right)

def parent(root, node):
    if root is None:
        return root
    elif (root.left is not None and root.left.val == node.val) or \
         (root.right is not None and root.right.val == node.val):
        return root
    else:
        if root.val < node.val:
            return parent(root.right, node)
        else:
            return parent(root.left, node)

def delete(root, node):
    p = parent(root, node)
    if node.left is None or node.right is None:
        replacement = node.left if node.left is None else node.right
        if p is None:
            root = replacement
        elif p.val < node.val:
            p.right = replacement
        else:
            p.left = replacement
    else:
        replacement = successor(root, node)
        succ_parent = parent(root, replacement)
        succ_parent.left = replacement.right
        node.val = replacement.val
    return root

Require code review in terms of correctness, style consistency, efficiency, code readability, and "pythonic"ness. (I, of course, welcome any other comments on the code as well)

I know that this a bit big to review completely. Reviewing only one or more methods will be appreciated.

Also, the WikiPage talks about designing persistent data structures. When should one consider such design and what are the advantages?

Github: https://github.com/abhisheksp/algorithms
(the repo has the test cases)

docstrings

Docstrings matter. Delete the cryptic #BST comment and spell it out in a PEP8 class docstring:

class Node:
"""Implements a Binary Search Tree."""

Either search or __init__ needs a docstring that mentions the equality case (I'm looking at elif root.val < k:, as opposed to <= k).

correctness

I can't imagine successor is correct. You didn't offer any unit tests like this, but I'm thinking of passing in a node with no right subtree, a node having the same value as root. Then we return None ?? This comes back to the <= k detail I mentioned above. Anyway, predecessor is nicely parallel, simple is good. And insert is beautiful and concise.

public API design

For some methods, rather than passing in root, it would be more natural to be working with an instance of your BST class, with self.root implicitly available.

style

As a trivial style detail, in parent you might use parentheses a level deeper to remove the backslash.

I didn't run flake8 myself, but overall it looks good. I have a weak preference for if node.left: instead of if node.left is not None:.

tests

I'm looking at this:

class BSTTest(unittest.TestCase):
    def test_recursive_search(self):
        root  = build_bst()
        actual = search(root, 35).val
        expected = 35
        self.assertEqual(expected, actual)

I would much rather see the method expressed concisely:

    def test_recursive_search(self):
        root  = build_bst()
        self.assertEqual(35, search(root, 35).val)

It might even come out as:

    def test_recursive_search(self):
        self.assertEqual(35, search(self.root, 35).val)

where build_bst() is invoked by a setup method.

There are other tests, e.g. test_insert_on_empty_tree, where of course hanging on to an actual temp variable is the natural way to express it.

I am very glad to see these tests. They appear to give reasonable code coverage.

Another example tree that stores multiple copies of a given value (e.g. 30) would be of interest.

summary

Overall it looks good! Ship it.


Generating Table Sections from A Property in a List of Objects I'm working on a Xamarin application and I'm presenting a list of objects in a TableView that are grouped by a specific property of the object.

The property is a date. For each distinct date, I am creating a table section with a header containing said date. Below is the working code to accomplish this, but It's not very elegant.

    var trades = new List<ViewCell>();
    Dictionary<string, List<ViewCell>> dict = new Dictionary<string, List<ViewCell>>();

    // organizing each object into a dictionary with a key that
    // represents the table header and value for the sections objects
    foreach (var item in completed)
    {
        var date = item.DateClosed.ToString("MMMM dd yyyy");

        if (dict.ContainsKey(date))
        {
            dict[date].Add(new TradeCell(item));
        }
        else {
            dict[date] = new List<ViewCell> { new TradeCell(item) };
        }
    }

    // looping through the resulting dict to assemble the table
    foreach (KeyValuePair<string, List<ViewCell>> entry in dict)
        _table.Root.Add(new TableSection(entry.Key) { entry.Value });

My Question

This gets the job done but I was hoping for a more elegant approach, perhaps using linq?

You asked for a more elegant approach... This is certainly more concise:

var dict = completed
    .GroupBy(
        item => item.DateClosed.ToString("MMMM dd yyyy"), // Key selector
        item => new TradeCell(item)) // Value selector
    .ToDictionary(g => g.Key, g => g.ToList());

However, you don't actually need the dictionary (that's just to keep the same behaviour as the foreach). This should do the job:

foreach (var groupOfCells in completed.GroupBy(item => item.DateClosed.ToString("MMMM dd yyyy"), item => new TradeCell(item)))
{
    _table.Root.Add(new TableSection(groupOfCells.Key) { groupOfCells .ToList() });
}

Remember not to abbreviate names. dict doesn't mean anything to anyone.

I'd also suggest grouping by DateClosed.Date and converting to a string in your UI layer. I don't know if you need it, but it's much harder to localise if you include hard coded format strings all over the place.


CSV Reader from scratch I am new to Java and not quite familiar with its design patterns.

I have tried to implement a CSV-File reader from scratch. The CSVFile constructor accepts the path to the file and a class which represents each datapoint (line in file). This class must implement the "fromLine" method (which accepts an array of Strings (each element being in a different column)) and the "toLine" method (which turns an object into a CSV-File line (String)). Both are described in the CSVDataPoint interface.

import java.util.List;
import java.util.ArrayList;
import java.io.BufferedReader;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.io.FileReader;
import java.io.PrintWriter;

public class CSVFile
{
    Class dp;
    String path;

    public CSVFile(String path, Class<? extends CSVDataPoint> dp)
    {
        this.path = path;
        this.dp = dp;
    }

    public List load(int skip) throws Exception
    {
        List fields = new ArrayList<CSVDataPoint>();
        int lineCount = 0;

        try(BufferedReader br = new BufferedReader(new FileReader(path))){
            String line;
            while((line = br.readLine()) != null){
                lineCount++;

                if(lineCount > skip)
                {
                    CSVDataPoint a = (CSVDataPoint) dp.newInstance();
                    fields.add(a.fromLine(line.split("\\,")));
                }
            }
        }

        return fields;
    }

    public void dump(List<CSVDataPoint> lines) throws Exception
    {
        String result = "";
        PrintWriter writer = new PrintWriter(path, "UTF-8");

        for(int n = 0; n < lines.size(); n++)
        {
            writer.println(lines.get(n).toLine());
        }

        writer.close();
    }
}

CSVDataPoint interface:

public interface CSVDataPoint
{
    Object fromLine(String[] line);
    String toLine();
}

Although the code does work, I am not sure whether it is stylistically correct. Any criticism would be appreciated greatly.

Here are my comments in order of severity:

1) Bugs

1.1) resource handling

in load() you handled IO resources correctly with try-with-resources.  You forgot to do it in dump().

1.2) parsing

You split the line according to single comma. There are two problems with that: First, you do not accept cell values containing comma.  Imagine the following line

cell1,"cell with comma,",2018-05-31

Second, although the acronym stands for "comma separated values", parsers (and writers) usually accept any single character as argument for delimiter. this gives clients flexibility that sometimes is necessary. Imagine if an api produces  files in the following format

cell1;cell with comma,;2018-05-31

1.3) Instantiation of CSVDataPoint

Later on I have more (much more) to say about CSVDataPoint but this comment is relevant for the general case of instantiation through reflection: You make the assumption that the default no args constructor exists, and this is not always true. A better aproach would be for you to accept an instance of factory that produces instances of CSVDataPoint.

2) Warning

2.1) variable types

List fields = new ArrayList<CSVDataPoint>();
I know the compiler accepts this definition, but it makes no sense. What type do you want for the items in the list?  If you want it to accept any type use the ? wildcard (not Object)

Also, dp instance variable has no generic type.

2.2) unsued imports

you have unsued imports. clean code helps maintainability.

3) Design

3.1) CSVDataPoint

I have several comments regarding the class:

name
It took me some time to understand that this class represents a complete CSV line.  In my eyes, DataPoint translates to single cell.  Why not CSVDataLine or use the term from RDBMS CSVDataRow or the term from old times' file processing CSVDataRecord
toLine()
You do no validation on the lines that the method produces.  What if it did not produce comma delimited values?  the method should give you List<String> and you write the line in the proper format (perhaps with the custom delimiter?).
Instantiation
In load() you create a new instane of the class for every input line and dump() (also a bad name if you ask me) you accept a list of instances. from that I gather that the design was that a CSVDataPoint instance represents a CSV line. However, the usage contradicts the design.  fromLine() is supposed to produce an Object.  Did you mean it to return an instance of CSVDataPoint? in this case, a better approach would be to use a constructor.  This is the proper way for a Java bean to populate its state.

3.2) input to CSVFile

CSVFile only accepts file name in String.  this is too limited for two reasons: First, consider the case of a client that already constructed a File instance and now has to convert that to a String for you. The bigger issue is the case where the client already opened the file. also, the client may wish to have control over the lifecycle of the file.  This is of course true for file load and write. at the very least you should provide overloaded constructor that accepts File and one that acepts InputStream (and in this case do not close it).  You may ask why InputStream and not Reader.  well, you can have overloaded constructors for both. but I think an InputStream is enough. You can open a Reader from an InputStream.  InputStream is used in more cases than Reader.  For example, a common usage is to open one from an HTTP request.


Rails 3 nested controller method I have 2 nested resources in one of my Rails apps and the way the index is handled by the inner resources bothers me somewhat:

def index
    @program = Program.approved.find(params[:program_id]) if params[:program_id]
    if params[:tags]
      tags = params[:tags].split(" ")
      if @program
        @sites = Site.where(:program_id => @program.id).tagged_with(tags).page(params[:page])
      else
        @sites = Site.joins(:program => :user).tagged_with(tags).page(params[:page])
      end
    else
      if @program
        @sites = Site.where(:program_id => @program.id).page(params[:page])
      else
        @sites = Site.joins(:program => :user).page(params[:page])
      end
    end

    respond_to do |format|
      format.html # index.html.erb
      format.json { render json: @sites }
    end
end

The complexity is caused because I want my Site resource to be accessible both through it's Program or directly. The results are also optionally filtered based on tags if they are included in the params.

The joins are required because if a site belongs to a user then I display Edit/Delete options.

It just doesn't feel like good Ruby code. So any refactoring tips would be helpful.

You could get rid of the code duplication by making use of the fact, that arel expressions are lazily evaluated:

def index
  @program = Program.approved.find(params[:program_id]) if params[:program_id]

  sites =
    if @program
      Site.where(program_id: @program)
    else
      Site.joins(program: :user)
    end
  if params[:tags]
    tags = params[:tags].split(" ")
    sites = sites.tagged_with(tags)
  end
  @sites = sites.page(params[:page])

  respond_to do |format|
    format.html
    format.json { render json: @sites }
  end
end

Because you're using ruby 1.9 hash syntax in the render expression I also adapted the other hashes. That seems more consistent for me.


Calculate FFT from windowed time-based signal I have a time-based signal (Raw signal) sampled at 6 MHz and need to analyze it in freq. domain.
I'm learning DSP and this is very first time I work with DSP. Could you please help to check if this code is correct or not. And I have couple of questions needed to be clarify:

I've heard about linear scale and logarithm scale in order to observe the amplitude easier. Could you please explain to me, in this case, how can I get the logarithm scale?
for some function in Matlab (for example fft, spectrogram, etc), how can I get the code (just in case I want to change something to adapt my need)?

input=V5k_a100;
N=2048;     L=4096;    Fs=6e6;
spectrumTemp=[];

for index=1:length(input)/L
  slice = input(1+(index-1)*L:index*L);
  abcd = hann(L) .* slice;
  NFFT = 2^nextpow2(N);
  spectrumTemp(index, :) = fft(abcd, NFFT)/L;
end

f = [0:NFFT-1]*Fs/NFFT;
freq = f(1:length(f)/2);
time = [0:length(input)/L-1]*L/Fs;
spectrum = spectrumTemp(:, 1:length(f)/2);
mesh(time,freq',abs(spectrum'));

The easy way to plot something with a logarithmic scale would be to just take the logarithm of the variable.

logspectrum = log(spectrum);
The source of some files is available when you type edit function name, example:

edit unique

However, not all Matlab source is available, so unfortunately you cannot customize everything this easily.


Creating unique registration numbers using object factories I'm learning about immutability and uniqueness in Java, particularly the use of defensive programming and object factories.

I have been asked to create a class containing an object factory that when called creates a unique registration number comprised of a single letter and 4 digits. I think my solution ticks all boxes, but would appreciate some input on this one before I go any further to see if I'm on the right track.

public class RegistrationNumber
{
    private static final Map<String, RegistrationNumber> REG = new HashMap<String, RegistrationNumber>();

    private static int number;
    private static char letter;
    private static String strRep;

    private RegistrationNumber(int number, char letter, String strRep) {

      this.number = number;
      this.letter = letter;
      this.strRep = strRep;

    }

    public static final RegistrationNumber getInstance() {

       Random r = new Random();

       int numbers = r.nextInt(9000) + 1000;

       Character letter = (char)(r.nextInt(26) + 'a');

       String strRep = letter + "" + numbers;

       RegistrationNumber n = REG.get(strRep);

       if (n == null) {

           n = new RegistrationNumber(numbers, letter, strRep);

           REG.put(strRep, n);

        }
             return n;
           }

      char getLetter() {

          return letter;

        }

      int getNumbers() {

          return number;

        }

      public String toString() {

       return strRep;

    }

}

The number, letter, and strRep fields should be final and must not be static.  By making them static, you've made all of your RegistrationNumber objects very weird, and quite the opposite of immutable.  Constructing one RegistrationNumber changes every previously generated RegistrationNumber!

getInstance() is a bit odd.  Conventionally, the name getInstance() would retrieve a singleton instance — it should the same object every time it is called.  What you have instead is a method that usually generates a new object, but occasionally returns an existing object.  I'm puzzled by the motivation behind that behaviour.


Matlab IIR filter implementation I need to implement the Matlab filter(b,a,x) function in Java. Before I do this, I would like to rewrite the function in Matlab before translating into Java. So far, writing out all the coefficients works really well. See below:

x = load('data.csv');
windowSize = 10;
b = (1/windowSize)*ones(1,windowSize); % b coefficient - a is unused
i = 1;
for n = length(b) : length(x) % yes I know I am losing my first 10 samples
    y(i,:) = [ b(1)*x(n) + b(2)*x(n-1) + b(3)*x(n-2) + b(4)*x(n-3) + ...
        b(5)*x(n-4) + b(6)*x(n-5) + b(7)*x(n-6) + b(8)*x(n-7) + ...
        b(9)*x(n-8) + b(10)*x(n-9) ];
    i = i+1; % used for indexing calculated results into new array
end

However, I want the loop contents to automatically update when I change the window size. In the above example, if I were to increase the windowsize by a factor of two, then I would have to write out twice as many multiples of b and x within the for loop. Which is a terrible solution to my problem. Please advise.

So this solution works:

x = load('data.csv');
windowSize = 10;
b = (1/windowSize)*ones(1,windowSize);
i = 1;

for n = windowSize: length(x)
    m = 0;
    tempSum = 0;
        for j = 1 : windowSize
            temp = b(j)*x(n-m);
            tempSum = tempSum + temp;
            m = m + 1;
        end
    y(i,:) = [tempSum]
    i = i+1;
end

See results below:

Instead of having really inaccurate results in the beginning I just skip these. It's important to note that you are losing an amount of samples equal to the number of b coefficients in this solution. So make sure to rescale the X-axis.

Results were compared using Matlabs C = setdiff(y,yy) function and found no difference between the proposed solution and Matlabs filter(b,a,x) function.


Putting items next to each other I want to put some text next to its label (both of which are text blocks - which are faster than labels as I understand it). The best way I've found for doing this is a stack panel.

I have (repeated 3 times in a control) code like:

    <StackPanel  Grid.Column="1" Grid.Row="2" Grid.ColumnSpan="2" Orientation="Horizontal">
        <TextBlock Text="Start:" FontWeight="DemiBold" VerticalAlignment="Bottom" HorizontalAlignment="Left"/>
        <TextBlock  Text="12:00pm"  VerticalAlignment="Bottom" HorizontalAlignment="Left" Margin="5,0"/>
    </StackPanel>

Is this the cleanest way to do this, or is there a better control? I was kind of thinking about making a wrapper control around the stack panel, but then I would just end up with:

    <LabeledTextBlock Grid.Column="1" Grid.Row="2" Grid.ColumnSpan="2">
<LabledTextBlock.Label>
        <TextBlock Text="Start:" FontWeight="DemiBold" VerticalAlignment="Bottom" HorizontalAlignment="Left"/>
</LabledTextBlock.Label>
        <TextBlock  Text="12:00pm"  VerticalAlignment="Bottom" HorizontalAlignment="Left" Margin="5,0"/>
    </LabeledTextBlock >

I guess I could refine it more:

<LabeledContent Grid.Column="1" Grid.Row="2" Grid.ColumnSpan="2" Label="Start" Content="12:00pm"/>

Simplicity at the cost of flexibility. Perhaps instead I should be doing it as a style?

My opinion is that for simple tasks like in your situation, you should not complicate your XAML. Wrapping a StackPanel around two TextBlock-elements is still pretty clean.

Extracting the code and wrapping it in your own control where you would set the Label and Content surely is an option. But if this only to be done once, I think you're overdoing it. Only when you have to place the same code there a lot of times, your XAML will be cleaner using your own control.


Algorithm for Run Length Encoding - String Compression I attempted a problem from the Cracking the Coding Interview book. The following input: aabcccaaa should give the following output: a2b1c3a3. Any advice is much appreciated!

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

char* compress(char*);

int main(int argc, char** argv)
{
    if(argc==2)
    {
        printf("The compression of %s is %s \n", argv[1],
            compress(argv[1]));
    }
    else
        printf("Not correct number of inputs.");
}

char* compress(char* str)
{
    const size_t len = strlen(str);
    char result[len];

    //initialized to 0, so length check can be done later
    memset(result, 0, len);

    int counter;
    int j = 0;

    if(str==NULL || len == 0)
        return 0;

    result[0] = str[0];
    for(size_t i = 0; i < len; i++)
    {
        counter = 1;//reset counter

        //to make sure no array out of bounds exception occurs
        if(result[j+1]==0)
            result[++j] = counter + '0';
        else
            return str;

        while(str[i] == str[i+1])
        {
            //to store number in char array, add the asci value
            // of 0.
            result[j] = (++counter + '0');
            i++;
        }
        if(result[j+1]==0)
            result[++j] = str[i+1];
        else
            return str;
    }

    result[++j] = '0';

    //to avoid "function returns address of local variable" error
    //to also avoid altering original argument
    char* str1 = (char*)malloc(sizeof(result));
    strcpy(str1, result);
    return str1;
}

You can eliminate that function prototype by defining main() last, but it's up to you.
It's more common to first check the command line usage and then exit with a non-zero value if it's invalid.  You should also print all errors to stderr, not stdout (the latter is always used with printf()).

if (argc != 2)
{
    fprintf(stderr, "Not correct number of inputs.");
    return -1;
}

You can probably check str right at the start, especially with the len assignment.  If there is an error, then it may be clearer to return NULL instead of 0.  At least that's what many library functions return specifically.
The while loop can instead be a for loop:

for (; str[i] == str[i+1]; i++)
{
    //to store number in char array, add the asci value
    // of 0.
    result[j] = (++counter + '0');
}

The NULL character for result is wrong; it should be '\0'.
There's actually no need to cast the return type of malloc() in C.  Although malloc() does return a void*, it's not required because any pointer type can be assigned to a void* and is done "automatically" here.

Moreover, you can avoid that aforementioned error by allocating result after making sure that the original string is valid.  This will also avoid the need to call strcpy() at the end and risk other bugs.  Since result will not be local this way, you can return it from the function without losing the data.

char* compress(char* str)
{
    // check to make sure str is valid...

    // assuming len is also valid
    char* result = malloc(len);

    // OR, use calloc() instead to get the memory zeroed
    //   thus no need for memset() or anything else
    char* result = calloc(len, sizeof(char));

    // probably also good to check for sufficient memory
    // if insufficient, print error and abort
    if (!result)
    {
        perror("malloc"); // or "calloc"
        abort();
    }

    // do the compression...

    // return it right away; no need to copy anything
    return result;
}

You never call free() on str1, so this function leaks memory.  You would have to do this in main() with the returned string, and it may be necessary to assign compress() to a const char* to display before freeing afterward.  This also means that compress() should return a const char*, not a char*, preventing it from being modified further.

It may work to have something like this in main():

int main(int argc, char** argv)
{
    if (argc != 2)
    {
        // let the user know of the correct input
        fprintf(stderr, "usage: %s string\n", argv[0]);
        return -1;
    }

    const char* original = argv[1];
    const char* compressed = compress(original);

    printf("The compression of %s is %s \n", original, compressed);

    free(compressed);
}


Loop filling a set with logging After watching Raymond Hettingers talk Transforming Code into Beautiful, Idiomatic Python I got back to a function I wrote.

I'm not quite sure how to make it more pythonic but I think this might be a use-case to use the map function.

import logging
import string
import os

def _mounted_drives(possible_device_letters=string.ascii_uppercase):
    """Returns a set of mounted drive letters on a Windows machine."""
    result = set()
    for letter in possible_device_letters:
        mounted = os.path.exists(letter + ':')
        if mounted:
            logging.info('Found device ' + letter)
            result.add(letter)

    return result

if __name__ == '__main__':
    print(_mounted_drives())

Removing the logging and using set comprehension :

def _mounted_drives(possible_device_letters=string.ascii_uppercase):
    return {l for l in possible_device_letters if os.path.exists(l + ':')}

If you do want the logging but you do not care about it being done once every device is found :

def _mounted_drives(possible_device_letters=string.ascii_uppercase):
    s = {l for l in possible_device_letters if os.path.exists(l + ':')}
    for l in s:
        logging.info('Found device ' + l)
    return s


How to check if workbook not corrupted I have asked this on the main stackoverflow.com site, but I have been redirected here. My problem is, how to check, if a workbook that is downloaded from sharepoint is not corrupted and can be open. Well, the simplest possible way is to try to open it. I have written a function for that:

Function CanBeOpen(sPath As String) As Boolean
Dim wb As Workbook
On Error Resume Next

With Application
.EnableEvents = False
.DisplayAlerts = False

Set wb = .Workbooks.Open(sPath, False, True)
.SendKeys "~", True 'To close progressbar when opening

Select Case TypeName(wb)
    Case Is = "Nothing"
        CanBeOpen = False
    Case Is = "Workbook"
        CanBeOpen = True
    Case Else
        CanBeOpen = False
End Select

wb.Close savechanges:=False
.EnableEvents = True
.DisplayAlerts = True
End With

Set wb = Nothing
On Error GoTo 0
End Function

It works but I have strong feeling that this could be done in more efficient way. Do you have any suggestions?

There is Err object, that contains information about run-time errors:
https://msdn.microsoft.com/en-us/library/ka13cy19(v=vs.90).aspx

Err.Number represens error code, if no error Err.Number = 0

You can use this approach in your function:

On Error Resume Next
Set wb = .Workbooks.Open(sPath, False, True)
If Err.Number = 0 then
    wb.close
    CanBeOpen = true
else
    CanBeOpen = false
end if


Scraping news articles from various pages Scraping news articles from various web page is the main goal of this code. For starting all spiders(web crawlers) at the same time, I try to implement the facade design pattern. It works but I feel the constructor of SpiderFacade class can be improved. Is there better way to do it or better design pattern to do it ?

Abstract Spider

public abstract class NewsSpider {
    public void crawl() {
    }
}

Custom Spider1

public class NYTimesSpider extends NewsSpider {

   private final NewsService newsService;

   public NYTimesSpider(NewsService newsService) {
       this.newsService = newsService;
   }

   @Override
   public void crawl() {
       // crawl logic
   }
}

Custom Spider2

public class BbcSpider extends NewsSpider {

   private final NewsService newsService;

   public BbcSpider(NewsService newsService) {
       this.newsService = newsService;
   }

   @Override
   public void crawl() {
       // crawl logic
   }
}

Facade Class

public class SpiderFacade {

    private final List<NewsSpider> spiderList;

    public SpiderFacade(NewsService newsService) {
        spiderList = Arrays.asList(
                new NYTimesSpider(newsService),  // the wrong part in my opinion
                new BbcSpider(newsService)
        );
    }

    public void startCollectingNews() {
        spiderList.forEach(NewsSpider::crawl);
    }
}

Starting Spiders

 SpiderFacade spiderFacade = new SpiderFacade(newsService);
 spiderFacade.startCollectingNews();

Use interface instead of class with single method
Each concrete implementation should be a singletone
Each concrete implementation should contain only logic and accept your current news in crowler() method: do not recreate a strategy instance for each news.


Get tuple of the first and last days of the last N months I am trying to get a tuple of date (or datetime) objects over the last N months.

My thought was to use the dateutil package with something like this:

def last_n_months(n=12, ending=None):
    """Return a list of tuples of the first/last day of the month
    for the last N months
    """
    from datetime import date
    from dateutil.rrule import rrule, MONTHLY
    from dateutil.relativedelta import relativedelta

    if not ending:
        ending = date.today()

    # set the ending date to the last day of the month
    ending = ending + relativedelta(months=+1, days=-ending.day)

    # starting is the first day of the month N months ago
    starting = ending - relativedelta(months=n, day=1)

    months = list(rrule(MONTHLY, bymonthday=(1, -1), dtstart=starting,
                        until=ending))

    # we return pairs of dates like this: (1st day of month, last day of month)
    months = zip(months[::2], months[1::2])

    return months

Example usage:

    >>> from datetime import date, timedelta

    # get last two months as a degenerate example
    >>> l2n = last_n_months(2, ending=date(2012, 01, 01))
    >>> map(lambda x: [x[0].year, x[0].month, x[0].day], l2n)
    [[2011, 11, 1], [2011, 12, 1], [2012, 1, 1]]
    >>> map(lambda x: [x[1].year, x[1].month, x[1].day], l2n)
    [[2011, 11, 30], [2011, 12, 31], [2012, 1, 31]]

    >>> l24n = last_n_months(24, ending=date(2012,03,16))
    >>> len(l24n) # inclusive of current month
    25

    # every tuple starts with the first day of the month
    >>> all(x[0].day == 1 for x in l24n)
    True

    # every tuple ends with the last day of the month
    >>> all((x[1] +timedelta(days=1)).month != x[1].month for x in l24n)
    True

    # every tuple is the same month
    >>> all(x[0].month == x[1].month for x in l24n)
    True

I am posting it here to see if anyone has a better solution than this (and perhaps see if this yields some off-by-one sort of error that I haven't thought of).

Is there a simpler or faster solution than this?

def last_n_months(n=12, ending=None):
    """Return a list of tuples of the first/last day of the month
    for the last N months
    """
    from datetime import date
    from dateutil.rrule import rrule, MONTHLY
    from dateutil.relativedelta import relativedelta

It is better to import outside of the function. Typically imports go at the top of the file.

    if not ending:

You should check for none like: if ending is not None: just to be explicit about what you are checking for.

        ending = date.today()

    # set the ending date to the last day of the month
    ending = ending + relativedelta(months=+1, days=-ending.day)

Modifying ending rubs me the wrong way.

    # starting is the first day of the month N months ago
    starting = ending - relativedelta(months=n, day=1)

    months = list(rrule(MONTHLY, bymonthday=(1, -1), dtstart=starting,
                        until=ending))

    # we return pairs of dates like this: (1st day of month, last day of month)
    months = zip(months[::2], months[1::2])

    return months

You can combine these two lines


Speeding up brute force algorithm The code is no longer relevant but I want to know how I can make it faster. It uses only about 10% of my CPU. Any advice about best practices, better algorithms, or how to make the existing one faster are welcome. This is purely for the purpose of personal learning.

public class BruteForce {
public static void main(String[] args) throws Exception {

    // Password dictionary
    FileInputStream fstream = new FileInputStream("C:/list.txt");
    BufferedReader BufferedReader1 = new BufferedReader(new InputStreamReader(new DataInputStream(fstream)));
    // Read File Line By Line
    String input;
    long counter = 0;
    Long start = System.currentTimeMillis();
    while ((input = BufferedReader1.readLine()) != null) {
        if(counter%1000000 == 0){
            System.out.println(counter + ": " + input);
        }
        counter++;

        // Generate ID using published class
        byte[] pub = ppg.Indors.getPublicKey(input);
        // We could use getId, but we get a weird signed/unsigned problem
        byte[] pub2 = MessageDigest.getInstance("SHA-256").digest(pub);
        // Get account ID
        BigInteger BigInteger = new BigInteger(1, new byte[]{pub2[7],pub2[6],pub2[5],pub2[4],pub2[3],pub2[2],pub2[1],pub2[0]});

        // Request balance for ID
        String url = "http://localhost:1811/ppg?request=egtBalance=" + BigInteger.toString();
        URL obj = new URL(url);
        HttpURLConnection con = (HttpURLConnection) obj.openConnection();
        con.setRequestMethod("GET");
        BufferedReader BufferedReader2 = new BufferedReader(new InputStreamReader(con.getInputStream()));
        String inputLine;
        StringBuffer response = new StringBuffer();
        while ((inputLine = BufferedReader2.readLine()) != null) {
            response.append(inputLine);
        }
        BufferedReader2.close();
        // Check balance bigger than 0
        if (!response.toString().contains("balance\":0")) {
            System.out.println("Found:" + input);
        }
    }
    System.out.println("Speed (tries/sec): " + counter*1000/(System.currentTimeMillis()-start));
    BufferedReader1.close();
}
 }

A huge amount of you work is just plain work that has to be done.... (the joys of brute-forcing).

MessageDigest

On the other hand, this work is unnecessary:

byte[] pub2 = MessageDigest.getInstance("SHA-256").digest(pub);

This should be replaced with:

MessageDigest digest = MessageDigest.getInstance("SHA-256");

for (..... ) {

    digest.reset();
    byte pub2 = digest.digest(pub);

Accessing the security API's in every loop is a lot of wasted time, and MessageDigest classes are designed to be reused.

Parallelism...

A lot of the work you do involves getting and managing URL's... this is slow work that can be done in parallel with the hard work of generating the key hashes.

I would recommend one thread that builds a stream of hashes (with the relevant input stored) and feeds them in to a BlockingQueue with a limited queue size... then, have the controlling thread read these off the queue, and 'farm' them out to a number of other threads that create and check the URL's against the server.

The parallelsim could give you orders-of-magnitude performance improvements.... (but will heavily load the server you are bruting...)


Precise timing event dispatching in Java I have coded a tool to produce precise timing events in Java. You set a value in BPM, and you receive events in your listener at the most precise timing it possibly can be with Java.
It is basic. A thread, a loop, read the bpm value, invokes the listener (lambda), compute the time to sleep to match the bpm requested, and go on.
To achieve high precision, I sleep a bit less than calculated, and then have a noop loop that does the fine grain waiting.
It works well enough for my needs (less than 0.01BPM of error more than 95% of the time), but I was curious on how I could enhance it. Specificaly :

reduce the cpu usage
reduce the memory footprint
any coding that may permit to reduce the tolerance of the unit tests (see MAX_ERRORS_RATE_ALLOWED and BPM_TOLERANCE) without generating assertion errors
more or less any comment that is relevant to the context

The full project (maven) with very few dependencies is here: https://github.com/sebpiller/metronom
Here is the loop doing the computation:
    // https://github.com/sebpiller/metronom/blob/develop/src/main/java/ch/sebpiller/tictac/TicTac.java
    private void loopUntilStopped() {
        long n;
        int i=0;
        while (!stopped) {
            // ok, then tick and wait exactly the correct amount of time
            n = System.nanoTime(); // memorize last boom

            ///// Notify of tick
            ticTacListener.beat(i++ % 4 != 0, bpm);

            // next loop bpm
            bpm = bpmSource.getBpm();

            if (bpm <= 0) {
                // shutdown as soon as the bpm return 0
                stopped = true;
            } else {
                // compute next tick nanos:
                long nanosBetweenTicks = (long) (60_000_000_000d / bpm);

                /// How much time did we need to sleep until next boom
                long sleepNanos = n + nanosBetweenTicks - System.nanoTime() - __JAVA_CORRECTION_NANOS;

                if (sleepNanos < 0) {
                    LOG.warn("missed tic! ({}ns)", sleepNanos);
                    i++; // erf... let's hope we miss only one tick
                } else {
                    sleepInterrupted(sleepNanos / 1_000_000, (int) (sleepNanos % 1_000_000));

                    /* Just in  case this lady was really early this time...
                       fine tuning of the slept time: just loop doing nothing until we are close from the goal */
                    // this is where all the precision is really obtained... this wonderful, but empty loop !
                    while(System.nanoTime()<n + nanosBetweenTicks) /* no body here! */;
                }
            }
        }

A parameterized unit test checks the precision of the implementation here, and run the test with various tempo: https://github.com/sebpiller/metronom/blob/develop/src/test/java/ch/sebpiller/tictac/TicTacTest.java (the test should take about 10 minutes to complete in his current form)
Bonus: any specific advice regarding the execution of this code on an ARM32v7 board (RPi4)
Thank you in advance !

First of all, let me clarify that you can't do precise timing on a "normal" desktop or server operating system. You can be close, but the schedulers are not designed for that, you're not guaranteed to get a time slice for execution at all. The smaller your time-slices get, the bigger the error margins will be. The higher the load on the machine will be, the more slices you will miss.

 * Like my girlfriend, Java has some problems to be on time. He is always a bite late.

I'm most likely old and grumpy, but "funny" comments and variable names are a waste of everyones time at the end of the day. That and, if you ever want to use your code as portfolio for a job, or in your job, might cost you. Because when I see that comment, I can deduct a few things:

The programmer considers it okay to make fun of others in the public.
The programmer does not know anything about how the JVM works.
The programmer does not know anything about how the OS scheduler works.
The programmer does most likely not know anything about the Java ecosystem (Java is used for high-speed trading with stock exchanges, doing trades in the microsecond range).

Of course, what you do in your private projects is none of my business, but be aware that this comes at a cost in one way or another...and maybe not for you, as others might have to maintain your code at some point.
And to ask a different question...does your girlfriend know she's a cheap joke on the internet for you?

long n;
int i=0;

These names could be better, a lot better actually.

long nanosBetweenTicks = (long) (60_000_000_000d / bpm);

This would, do well as a constant.

__JAVA_CORRECTION_NANOS

Normally, you don't have leading underscores under the Java convention.

i++; // erf... let's hope we miss only one tick

That is what I was talking about earlier and is a very good example for a completely unhelpful comment. It tells me that the programmer either had no idea what they were doing, or did not care enough to check it.

while(System.nanoTime()<n + nanosBetweenTicks) /* no body here! */;

I believe that this will not be optimized away because of the System.nanoTime() function call, but I would not bet on it.

The code is otherwise well formatted, and most of the names are good. I'm not sure about the rest of the logic, it seems oddly complicated for what you describe.


Doing a qsort function to compare strings, case insensitive How does the following C program look to compare to strings and return a case-insensitive sort? The following is what I have so far:
int main(void)
{
    char* strings[4] = {"Onus", "deacon", "Alex", "zebra"};
    for (int i=0; i<4; i++) printf("%d. %s\n", i+1, strings[i]);
    qsort(strings, 4, 8, scmp);
}

int scmp(const void *p1, const void *p2)
{

    // being a string (pointer-to-char)
    const char* s1 = *(char**) p1;
    const char* s2 = *(char**) p2
    size_t len_s1 = strlen(s1);
    size_t len_s2 = strlen(s2);

    // lower-case first string buffer
    char s1_lower[len_s1+1];  // cannot initialize with {[len_s1]=0} in VLA ?
    s1_lower[len_s1] = 0;
    for (int i=0; s1[i] != 0; i++)
        s1_lower[i] = tolower(s1[i]);

    // lower-case second string buffer
    char s2_lower[len_s2+1];
    s2_lower[len_s2] = 0;
    for (int i=0; s2[i] != 0; i++)
        s2_lower[i] = tolower(s2[i]);

    // built-in string compare (strcmp) will return the sort value
    return strcmp(s1_lower, s2_lower);
    /* printf("%s -- %s\n", s1_lower, s2_lower); */

}

Another version extracting out the str_lower function would be:
void lower_string(char buffer[], const char* str, size_t len)
{
    char str_lower[len+1];
    str_lower[len] = 0;
    for (int i=0; str[i] != 0; i++)
        str_lower[i] = tolower(str[i]);
}
int scmp(const void *p1, const void *p2)
{
    const char* s1 = *(char**) p1;
    const char* s2 = *(char**) p2;

    char s1_lower[strlen(s1)+1];
    lower_string(s1_lower, s1, strlen(s1));

    char s2_lower[strlen(s2)+1];
    lower_string(s2_lower, s2, strlen(s2));

    return strcmp(s1_lower, s2_lower);
}

Firstly, thank you for providing a test program.  We could make the test even better, if we make it self-checking:
char const *expected[] = {"Alex", "deacon", "Onus", "zebra"};
for (int i = 0;  i < 4;  ++i) {
    if (strcmp(expected[i], strings[i])) {
        return EXIT_FAILURE;
    }
}

We might consider including some non-alphabetic characters, too ("should "A-team" sort before or after "abacus"?).  And we'll want some tests of equal and almost-equal strings; not just ones that differ in the first character.

There were a few errors and warnings to fix up (I compiled with gcc -std=c17 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds  -Wstrict-prototypes -Wconversion to catch all of these):

We missed a few necessary includes for the comparison function:
#include <ctype.h>
#include <stdint.h>
#include <string.h>

And for the test program:
#include <stdio.h>
#include <stdlib.h>

We're assigning char* pointers using string literals.  Whilst C allows this for historical reasons, it's dangerous, and should be avoided (because assignment through such pointers is UB, and the compiler can't spot that for you).  It's easy to fix that:
const char* strings[4] = {"Onus", "deacon", "Alex", "zebra"};

And one that GCC can't spot:

qsort(strings, 4, 8, scmp);
//               ^^^

We can't assume that a const char* is a particular size on the target platform.  Whilst this value may be correct where you are, it's not true everywhere.  Luckily, C provides the sizeof operator to help:
qsort(strings, 4, sizeof strings[0], scmp);

Looking in detail at the comparison function, it does a lot of extra work to make a copy of each input string (we have to traverse each string up to three times - once to find its length, once to copy and convert case, and once to compare).
Instead of preparing the ground for strcmp(), we could (and should) write a case-insensitive version of strcmp(), so we traverse each string once, and downcase only those characters we actually need for the comparison, as we reach them:
#include <ctype.h>

/* N.B. name not beginning with "str". */
int ci_strcmp(const char *s1, const char *s2)
{
    while (*s1) {
        if (!*s2) {
            /* s1 is longer */
            return 1;
        }
        int c1 = tolower(*s1);
        int c2 = tolower(*s2);
        if (c1 < c2) {
            return -1;
        }
        if (c2 < c1) {
            return 1;
        }
        ++s1;
        ++s2;
    }
    if (*s2) {
            /* s2 is longer */
            return -1;
    }
    /* compare equal */
    return 0;
}

/* Adapter for qsort */
int scmp(const void *p1, const void *p2)
{
    const char *const *sp1 = p1;
    const char *const *sp2 = p2;
    return ci_strcmp(*sp1, *sp2);
}

We can make the body of the comparer more compact (but functionally identical) with a little cleverness:
int ci_strcmp(const char *s1, const char *s2)
{
    for (;;) {
        int c1 = tolower(*s1++);
        int c2 = tolower(*s2++);
        int result = (c1 > c2) - (c1 < c2);
        if (result || !c1) return result;
    }
}

As mentioned in another answer, we need to use unsigned char with tolower():
int ci_strcmp(const unsigned char *s1, const unsigned char *s2)
{
    for (;;) {
        int c1 = tolower(*s1++);
        int c2 = tolower(*s2++);
        int result = (c1 > c2) - (c1 < c2);
        if (result || !c1) return result;
    }
}

/* Adapter for qsort */
int scmp(const void *p1, const void *p2)
{
    const unsigned char *const *sp1 = p1;
    const unsigned char *const *sp2 = p2;
    return ci_strcmp(*sp1, *sp2);
}


Sentence maker with thesaurus I'm teaching myself Ruby by rewriting PHP solutions, such as this one. The program should match words in a sentence with a dictionary of synonyms and then make combinations with them.

dict = {
    'hello' => ['hey', 'what\'s up'],
    'name' => ['nomenclature', 'epithet']
}

str = 'hello, my name is'

class SentenceMaker
    def initialize str, dict
        @thesaurus = []
        @template = str.gsub(/\w+/) {|match|
            if (dict[match])
                @thesaurus.push dict[match]
                '%%%d$s' % @thesaurus.length
            else
                match
            end
        }
    end

    def each
        @thesaurus.first.product(*@thesaurus.slice(1..-1)).each {|repl|
            yield @template % repl
        }
    end
end

SentenceMaker.new(str, dict).each {|sentence|
    puts sentence
}

Example output

hey, my nomenclature is
hey, my epithet is
what's up, my nomenclature is
what's up, my epithet is

Does this make enough use of the language expressions, or did I miss a few things?

Overall, it's pretty good. My comments are:

Use do/end for multiline blocks.
Typical Ruby style is a 2-space indent.
I think @thesaurus.drop(1) looks a bit better than @thesaurus.slice(1..-1)
match is not a good local variable name, word is more descriptive
Template is too complicated: you can just use multiple %s placeholders with an array of Strings.

I'm not commenting too much on the algorithm, since you weren't asking, but it seems a bit complicated. I'd be tempted to try simplifying it a bit.

Here's your code with the above changes applied:

dict = {
  'hello' => ['hey', 'what\'s up'],
  'name' => ['nomenclature', 'epithet'],
}

str = 'hello, my name is'

class SentenceMaker
  def initialize str, dict
    @thesaurus = []
    @template = str.gsub(/\w+/) do |word|
      if (dict[word])
        @thesaurus.push dict[word]
        '%s'
      else
        word
      end
    end
  end

  def each
    @thesaurus.first.product(*@thesaurus.drop(1)).each do |repl|
      yield @template % repl
    end
  end
end

SentenceMaker.new(str, dict).each {|sentence|
  puts sentence
}


KNN algorithm implemented in Python This is the first time I tried to write some code in Python.
I think it gives proper answers but probably some "vectorization" is needed

import numpy as np
import math
import operator

data = np.genfromtxt("KNNdata.csv", delimiter = ',', skip_header = 1)
data = data[:,2:]

np.random.shuffle(data)
X = data[:, range(5)]
Y = data[:, 5]

def distance(instance1, instance2):
    dist = 0.0
    for i in range(len(instance1)):
        dist += pow((instance1[i] - instance2[i]), 2)
    return math.sqrt(dist)

# Calculating distances between all data, return sorted  k-elements list (whole element and output)
def getNeighbors(trainingSetX, trainingSetY, testInstance, k):
    distances = []
    for i in range(len(trainingSetX)):
        dist = distance(testInstance, trainingSetX[i])
        distances.append((trainingSetX[i], dist, trainingSetY[i]))
    distances.sort(key=operator.itemgetter(1))
    neighbour = []
    for elem in range(k):
        neighbour.append((distances[elem][0], distances[elem][2]))
    return neighbour

#return answer
def getResponse(neighbors):
    classVotes = {}
    for x in range(len(neighbors)):
        response = int(neighbors[x][-1])
        if response in classVotes:
            classVotes[response] += 1
        else:
            classVotes[response] = 1
    sortedVotes = sorted(classVotes.items(), key=operator.itemgetter(1), reverse = True)
    return sortedVotes[0][0]

#return accuracy, your predicitons and actual values
def getAccuracy(testSetY, predictions):
    correct = 0
    for x in range(len(predictions)):
        if testSetY[x] == predictions[x]:
            correct += 1
    return (correct / (len(predictions))) * 100.0

def start():
    trainingSetX = X[:2000]
    trainingSetY = Y[:2000]
    testSetX = X[2000:]
    testSetY = Y[2000:]

    # generate predictions
    predictions = []
    k = 4
    for x in range(len(testSetX)):
        neighbors = getNeighbors(trainingSetX, trainingSetY, testSetX[x], k)
        result = getResponse(neighbors)
        predictions.append(result)
    accuracy = getAccuracy(testSetY, predictions)
    print('Accuracy: ' + str(accuracy))

start()

First a style nitpick: Python has an official style-guide, PEP8, which recommends using lower_case_with_underscores for variable and function names instead of camelCase.

Second, the comments you have above your functions should become docstrings.
These appear for example when calling help(your_function) in an interactive session. Just Have a string as the first line below the function header like so:

def f(a, b):
    """Returns the sum of `a` and `b`"""
    return a + b

It is recommended to always use triple double-quotes (i.e. """).

Now I am going to focus on the distance calculation.

First, you can greatly simplify your getNeighbors function using list comprehensions:

def getNeighbors(trainingSetX, trainingSetY, testInstance, k):
    distances = sorted((distance(testInstance, x), x, y)
                       for x, y in zip(trainingSetX, trainingSetY)
    return [(d[1], d[2]) for d in distances[:k]]

Here I used the fact that tuples already sort naturally, by first comparing the first index then (if they are equal) the second and so on. So I put the distance as the first index of the tuple and you don't need the key function any longer. sorted can take a generator expression and sort it directly. We can also iterate over multiple iterables at the same time using zip.

Since your variables are all numpy arrays, you could also do it more vectorized. For this I would first re-define the distance function to use numpy functions:

def distance(x, y):
    return np.sqrt(((x - y)**2).sum())

And then put the distances into a numpy array as well. Only returning the second and third column becomes then easier with array slicing.

def getNeighbors(trainingSetX, trainingSetY, testInstance, k):
    distances = np.array([(distance(testInstance, x), x, y)
                          for x, y in zip(trainingSetX, trainingSetY])
    distances.sort()
    return distances[:k, 1:]

This can probably be modified even further by trying to make the distance call vectorized as well.

Your function classVotes can be simplified using the collections.Counter class which implements exactly what you do her:

def getResponse(neighbors):
    classVotes = Counter(int(neighbor[-1]) for neighbor in neighbors)
    return max(classVotes.iteritems(), key=itemgetter(1))[0]

Your function getAccuracy can be slightly simplified using a generator expression and sum:

def getAccuracy(testSetY, predictions):
    correct = sum(y == p for y, p in zip(testSetY, predictions))
    return correct * 100.0 / len(predictions)

And lastly, in your start function, you can directly iterate over the elements of testSetX, make it a generator expression and use the fact that print can take multiple arguments:

def start():
    trainingSetX = X[:2000]
    trainingSetY = Y[:2000]
    testSetX = X[2000:]
    testSetY = Y[2000:]

    # generate predictions
    k = 4
    predictions = (getResponse(getNeighbors(trainingSetX, trainingSetY, x, k)]
                   for x in testSetX)
    accuracy = getAccuracy(testSetY, predictions)
    print('Accuracy:', accuracy)


Game of Life search program for Herschel-generating baits Golly is a cross-platform, Python and Perl extendable (like in this case, Python), free Conway's Game of Life simulator written in C++ (or Python) that runs in arbitrary speed and large, marvelous patterns.

Herschel is a pattern named after William Herschel (the heptomino resembles Saturn's planetary symbol ♄, and somewhere in the naming process Saturn got confused with Uranus, which was discovered by Herschel). Herschels frequently appear in the evolution of chaotic patterns.

The following script is a Golly script that searches for positions where one in a list of still lifes can create a Herschel (or another pattern specified in the "herschels" list) when reacting with the pattern already in the board.

import golly as g
from sys import exit

herschels = []

testrect = g.getselrect();
if testrect == []:
    g.exit("Select the area where the still life is placeable!")

if g.empty():
    g.exit("There is no pattern to add bait!")

stillLifes = []

def addStillLife():
    while True:
        stillLife = g.getstring("What still life to test for? (RLE)", "2o$2o!", "HRF v0.1")
        stillLifes.append(g.parse(stillLife))
        addAnother = g.getstring("Add another still life? (y|n)", "n", "HRF v0.1")
        if addAnother.lower() == "n":
            break
        elif addAnother.lower() != "y":
            g.exit("Invalid letter!")

def removeFirstLine(str):
    result = str.rsplit('\n')
    result.pop(0)
    return ''.join(result)

def addResultDesired():
    while True:
        herschelDesired = g.getstring("What result to find? (RLE)", "3o$obo$obo!", "HRF v0.1")
        herschels.append(g.parse(herschelDesired))
        addAnother = g.getstring("Add another result desired? (y|n)", "n", "HRF v0.1")
        if addAnother.lower() == "n":
            break
        elif addAnother.lower() != "y":
            g.exit("Invalid letter!")

addStillLife()
addResultDesired()
debug = g.getstring("Debug mode? (y|n)", "n", "HRF v0.1")
if debug.lower() == "y":
    g.autoupdate(True)
elif debug.lower() != "n":
    g.exit("Invalid letter!")

def testReaction(xpos, ypos, sli):
    orig = g.getcells(g.getrect())
    g.putcells(stillLifes[sli], xpos, ypos)
    this = g.getcells(g.getrect())
    i = 0
    oldgeneration = g.getgen()
    while i < 301:
        g.step()
        for y in herschels:
            if g.getcells(g.getrect()) == y:
                break
        i += 1
    if i == 301:
        g.show("No result found at x:" + str(xpos) + ", y:" + str(ypos) + ", still life " + str(sli))
    g.clear(0)
    g.clear(1)
    if i == 301:
        this = g.putcells(orig)
    else:A
        g.show("Found reaction!")
        g.putcells(this)
    g.setgen(oldgeneration)

n = 0
while n < testrect[2]:
    m = 0
    while m < testrect[3]:
        o = 0
        while o < len(stillLifes):
            testReaction(n + testrect[0], m + testrect[1], o)
            o += 1
        m += 1
    n += 1

Iterations

The pythonic way to iterate is to use the for keyword, usually without bothering about indices. I suggest you have a look at the presentation : Ned Batchelder: Loop like a native: while, for, iterators, generators (PyCon US) (video on Youtube/text with slides).

In your case, most loops can be rewritten. For instance :

n = 0
while n < testrect[2]:
    m = 0
    while m < testrect[3]:
        o = 0
        while o < len(stillLifes):
            testReaction(n + testrect[0], m + testrect[1], o)
            o += 1
        m += 1
    n += 1

could be written :

for n in range(testrect[2])
    for m in range(testrect[3]):
        for o, _ in enumerate(stillLifes):
            testReaction(n + testrect[0], m + testrect[1], o)

Also, the for loop accepts an optional else block after it which is called when the loop exited with no break (just because the thing we are iterating over has been fully consumed). This can be useful to rewrite :

i = 0
while i < 301:
    g.step()
    for y in herschels:
        if g.getcells(g.getrect()) == y:
            break
    i += 1
if i == 301:
    g.show("No result found at x:" + str(xpos) + ", y:" + str(ypos) + ", still life " + str(sli))

in a more straightforward way (and without repeating 301 in 2 places) :

for i in range(300)
    g.step()
    for y in herschels:
        if g.getcells(g.getrect()) == y:
            break
else:
    g.show("No result found at x:" + str(xpos) + ", y:" + str(ypos) + ", still life " + str(sli))

Disclaimer for the snipper above: I may have misread your original code so this might be wrong.

User input

At the moment, there are a few issues with the way you deal with user input.

you have to provide "HRF v0.1" several times.

You could write a function to hide implementation details and remove duplicated logic :

def get_user_string(prompt, initial):
    title = "HRF v0.1"
    return g.getstring(prompt, initial, title)

You are calling lower() on the same string multiple times. If only the lowercase version of the string is interesting to you, you could simple do : addAnother = get_user_string("Add another result desired? (y|n)", "n").lower().
You have logic to ask boolean input and check it in multiple places
Your program stops in case of invalid logic, it might be better to ask the user to re-input something.

In order to fix these issues, you could define something like :

def get_user_string(prompt, initial):
    title = "HRF v0.1"
    return g.getstring(prompt, initial, title)

def get_user_bool(prompt, initial):
    while True:
        string = get_user_string(prompt, initial).lower()
        if string == "n":
            return False:
        elif string == "y":
            return True
        else:
            golly.error("Invalid input.")

That you could use like this :

    addAnother = get_user_bool("Add another result desired? (y|n)", "n")
    if not addAnother:
        break

or this :

if get_user_bool("Debug mode? (y|n)", "n"):
    g.autoupdate(True)

This shows a better Separation of concerns.

Code organisation

Instead of dealing with global variables, it would be cleaner to try to find alternatives. In your case, using function argument and return values should make things clearer.

You could define :

def get_user_still_lifes():
    stillLifes = []
    while True:
        stillLife = get_user_string("What still life to test for? (RLE)", "2o$2o!")
        stillLifes.append(g.parse(stillLife))
        if not get_user_bool("Add another still life? (y|n)", "n"):
            break
    return stillLifes

def get_user_desired_results():
    herschels = []
    while True:
        herschelDesired = get_user_string("What result to find? (RLE)", "3o$obo$obo!")
        herschels.append(g.parse(herschelDesired))
        if not get_user_bool("Add another result desired? (y|n)", "n"):
            break
    return herschels

and use it like this :

stillLifes = get_user_still_lifes()
herschels = get_user_desired_results()

Also it is a good habit to put the part of your code actually doing things behind an if __name__ == "__main__" guard.

Style

Python has a style guide called PEP 8. It is an interesting read and it is probably worth trying to follow it if you don't have a good reason not to. For instance, your code does not follow the naming convention. You'll find various tools to check your code compliancy to PEP 8.

Final (untested) code

The following code has not been tested, feel free to edit it to fix errors and typos I may have introduced.

import golly as g
from sys import exit

def get_user_string(prompt, initial):
    title = "HRF v0.1"
    return g.getstring(prompt, initial, title)

def get_user_bool(prompt, initial):
    while True:
        string = get_user_string(prompt, initial).lower()
        if string == "n":
            return False:
        elif string == "y":
            return True
        else:
            golly.error("Invalid input.")

def get_user_rectangle():
    while True:
        rect = g.getselrect();
        if rect == []:
            golly.error("Select the area where the still life is placeable!")
        elif g.empty():
            g.error("There is no pattern to add bait!")
        else:
            return rect

def get_user_still_lifes():
    still_lifes = []
    while True:
        stillLife = get_user_string("What still life to test for? (RLE)", "2o$2o!")
        still_lifes.append(g.parse(stillLife))
        if not get_user_bool("Add another still life? (y|n)", "n"):
            break
    return still_lifes

def get_user_desired_results():
    herschels = []
    while True:
        herschelDesired = get_user_string("What result to find? (RLE)", "3o$obo$obo!")
        herschels.append(g.parse(herschelDesired))
        if not get_user_bool("Add another result desired? (y|n)", "n"):
            break
    return herschels

def test_reaction(still_lifes, herschels, xpos, ypos, sli):
    orig = g.getcells(g.getrect())
    g.putcells(still_lifes[sli], xpos, ypos)
    this = g.getcells(g.getrect())
    oldgeneration = g.getgen()
    for i in range(300)
        g.step()
        for y in herschels:
            if g.getcells(g.getrect()) == y:
                break
    else:
        g.show("No result found at x:" + str(xpos) + ", y:" + str(ypos) + ", still life " + str(sli))
    g.clear(0)
    g.clear(1)
    if i == 301:
        this = g.putcells(orig)
    else:A
        g.show("Found reaction!")
        g.putcells(this)
    g.setgen(oldgeneration)

if __name__ == "__main__":
    testrect = get_user_rectangle()
    still_lifes = get_user_still_lifes()
    herschels = get_user_desired_results()
    if get_user_bool("Debug mode? (y|n)", "n"):
        g.autoupdate(True)

    for n in range(testrect[2])
        for m in range(testrect[3]):
        for o, _ in enumerate(still_lifes):
            test_reaction(still_lifes, herschels, n + testrect[0], m + testrect[1], o)


How to make a program that lies one-third of the time? I am supposed to take this code (that I made):

public void showUpdatedStatus()
{
    if(((itsUsersNumber - itsSecretNumber) <= 2) && ((itsUsersNumber - itsSecretNumber) >= -2))
        JOptionPane.showMessageDialog(null, "You're hot!");
    if (((itsUsersNumber - itsSecretNumber) <= 6) && ((itsUsersNumber - itsSecretNumber) >= -6))
        JOptionPane.showMessageDialog(null, "You're warm.");
    if ((itsUsersNumber - itsSecretNumber) < -6)
        JOptionPane.showMessageDialog(null, "Too low.");
    if ((itsUsersNumber - itsSecretNumber) > 6)
        JOptionPane.showMessageDialog(null, "Too high.");
}

And change it so it does this:

  Have the program lie one-third of the time that the guess is too high;
  it says it is too low. But it never lies about the guess being too
  low, nor does it lie twice in a row.

I believe I have managed to do this with this method for checking whether the guess is "too high."

public void TooHigh()
{
    int numLies = 1;
    if ((itsUsersNumber - itsSecretNumber) > 6)
    {
        itsLieNum = randy.nextInt(2);
        if(itsLieNum == 1 && numLies == 1){
            numLies = 2;
            JOptionPane.showMessageDialog(null, "Too low.");}
        else
            numLies = 1;
            JOptionPane.showMessageDialog(null, "Too high.");
    }
}

I just want to know whether or not it seems like it would work fine. I would just test it, but it would probably take me quite a while to make sure that it not only works 1/3 of the time, but also that it doesn't do it twice in a row.

EDIT: I managed to get everything working fine with this code:

public void showUpdatedStatus()
{

    if(((itsUsersNumber - itsSecretNumber) <= 2) && ((itsUsersNumber -itsSecretNumber) >= -2))
        JOptionPane.showMessageDialog(null, "You're hot!");
    else if (((itsUsersNumber - itsSecretNumber) <= 6) && ((itsUsersNumber -itsSecretNumber) >= -6))
        JOptionPane.showMessageDialog(null, "You're warm.");
    if ((itsUsersNumber - itsSecretNumber) < -6)
        JOptionPane.showMessageDialog(null, "Too low.");
    else
        TooHigh();
}

public void TooHigh()
{
    if ((itsUsersNumber - itsSecretNumber) > 6)
    {
        itsLieNum = randy.nextInt(3);
        if(canLie && itsLieNum == 1){
            canLie = false;
            JOptionPane.showMessageDialog(null, "Too low.");}
        else{
            canLie = true;
            JOptionPane.showMessageDialog(null, "Too high.");}
    }
}

Thanks to rolfl

The concept you have going is 'OK' (cumbersome, but for a beginner it's OK). There are two bugs, and one recommendation that will help a lot.

First the recommendation....

Change numLies to be a boolean value like: boolean canlie = true;

Then, your code becomes:

if(canlie && itsLieNum == 1){
    canlie = false;
    ....
} else {
    canlie = true;

This makes the purpose of the code more apparent. Also, a bug is that you have to declare this ourside your method. It should be a class-variable, not a method variable, otherwise it will always be the wrong value.

The bug you have is a technical one (but important that you learn) ... Random.nextInt(value) will never return the input value. It will retrun 0(inclusive) to value(exclusive). In your case, with nextInt(2) it will only ever return 0, or 1. Change that to be nextInt(3) and you will get the values 0, 1, or 2 and it will be 1 1 third of the time.


Balancing Brackets Algorithm: Stack Data Structure I am currently refining my understanding of data structures and as such, decided to implement a popular application of the Stack. Below you will find my implementation of the Balancing Symbols algorithm written in JavaScript.

My basic questions are:

How clean would you rate my code?
Is time complexity O(n): Linear?
How can I improve?

function isBalanced(arr){
  var stack = new Stack();
  var last;

  arr.forEach(function(curr){
    if (curr === '(' || curr === '[' || curr === '{') {
      stack.push(curr);
    }

    if (curr === ')' || curr === ']' || curr === '}') {
      last = stack.peek() + curr;
      if (last === '()' || last === '[]' || last === '{}') {
        stack.pop();
      }
    }
  });

  var result = (stack.length() === 0) ?  true : false;
  console.log(result);
}

var symbols = ['{', '(', ')', ']'];

isBalanced(symbols);

The code looks reasonably clean, IMO. I assume that it is purposely ES5?

I don't know about Stack as it's not a ES5 standard API (that I know of).

You are not returning any value.  There is no need for ? true : false — just do return stack.length() === 0;.

Some code that you can consider, in ES5.

var completes = ['()', '[]', '{}'];
var open = completes.map(function(complete) {
  return complete.charAt(0);
});
var close = completes.map(function(complete) {
  return complete.charAt(1);
});

function isBalanced(arr) {
  var stack = [];
  arr.forEach(function(curr) {
    if (open.indexOf(curr) !== -1) {
      stack.push(curr);
    }
    if (close.indexOf(curr) !== -1) {
      var last = stack.pop();
      if (completes.indexOf(last + curr) === -1) {
        stack.push(last);
      }
    }
  });
  return stack.length === 0;
}

console.log(isBalanced(['{', '(', ')', ']']));
console.log(isBalanced(['[', '(', ')', ']']));


Determining if a target string is found in an array of strings I wrote this simple function in CoffeeScript to see if elements are found in an array:

isFoundIn = (searchTerm, arrayToSearch) ->
  returnVal = false
  for a in arrayToSearch
    returnVal = true if searchTerm is a
  returnVal

Usage:

isFoundIn('james', ['robert', 'michael', 'james'])`

would return true.

I know in Ruby it is very concise and readable to do this:

myArr = ['robert', 'michael', 'james']
myArr.include? 'james'

Are there some strategies to refactor this to make it more readable or more like CoffeeScript?

Expanding on my comment above:

Your isFoundIn function can be rewritten as simply

isFoundIn = (term, array) -> array.indexOf(term) isnt -1

indexOf uses the strict === comparison, which is what CoffeeScript also uses for is - i.e. it's equivalent to your code.

One could extend the native JS Array prototype with a include function to more closely mimic Ruby. In CoffeeScript, it'd be written as

Array::include = (term) -> @indexOf(term) isnt -1

Then you can do [1, 2, 3].include(3) and such. But extending native prototypes is generally frowned upon, and I can't recommend it. Tempting though.

As for your original function, you could just do an explicit return right away if the searchTerm is found - no need to loop through the rest of the array

    isFoundIn = (searchTerm, arrayToSearch) ->
      for a in arrayToSearch
        return true if searchTerm is a
      false

The point is moot, though, as indexOf does it all for you

Addendum: I do like the ? hack you used to fully match Ruby's method name, but it is still just that: A hack. You're effectively adding undefined as a possible return value for what should be a straight boolean. It'll also absorb errors (as that's its purpose) where e.g. the object responds to indexOf just fine, but - for whatever reason - hasn't been extended with the include function. So it's easy to get false negatives (unless you explicitly start checking for undefined and branch if that's the case and... well, the code fogs up quick)

So again, definite points for creativity - wish I'd thought of it - but I personally wouldn't go near it in production code


Haskell Parsec parser of Verilog-style number literals I've set myself the task to write a function that parses Verilog-style number literals. In Verilog, numbers are written like this:
8'b10101100, 16'hffff, '789, 32'd0, 12'o7, etc.

The syntax is:

<size>'<radix>value

where <...> denotes an optional field, and

size is the binary representation length of the number (in bits). The default is 32.
radix is 'b' or 'B' for binary, 'o' or 'O' for octal, 'd' or 'D' for decimal, 'h' or 'H' for hexadecimal. The default is 'd'.
value is the appropriate number representation for the given radix.

I'm new to Haskell, so I really would appreciate a critical appraisal of the following implementation. I think it is correct. Is it also 'good' Haskell/Parsec? Can it be done better, i.e. more concisely, more clearly, using more appropriate idioms?

import System.IO
import System.Environment
import Data.Either (either)
import Data.Either.Extra (fromRight)
import Data.Char (digitToInt, toLower)
import Numeric (readInt, readOct, readHex)
import Text.Parsec.Combinator
import Text.ParserCombinators.Parsec
import Text.Parsec.Char (hexDigit, octDigit)

integerLog :: Integer -> Integer
integerLog = floor . logBase 2.0 . fromIntegral

-- |numberSize calculates the length (in bits) of the binary representation of Integer
numberSize :: Integer -> Integer
numberSize = (1+) . integerLog

-- |The type Number has a val (Integer value) and a len (binary representation length).
data Number = Number {val::Integer, len:: Integer} deriving Show

-- |makeNumber is a safe constructor of an Either String Number
makeNumber :: Integer -> Integer -> (Either String Number)
makeNumber v l
  | v < 0     = Left "makeNumber: Negative values not allowed in Number"
  | size > l  = Left $ "makeNumber: Value " ++ show v ++ " is "
    ++ show size ++ " bits long, should be at most " ++ show l ++ " bits"
  | size <= 0 = Left $ "makeNumber: Size can't be negative or zero"
  | otherwise = Right $ Number v l
  where size = numberSize v

-- |readBin is not defined in Numeric (readInt, etc., are)
readBin = readInt 2 (`elem` "01") digitToInt

-- conversions String -> Number
hexToInteger x = fst $ readHex x !! 0 :: Integer
octToInteger x = fst $ readOct x !! 0 :: Integer
binToInteger x = fst $ readBin x !! 0 :: Integer

-- |binDigit is not in Text.Parsec.Char (hexDigit, octDigit are)
binDigit :: CharParser st Char
binDigit = oneOf "01"

-- |parse Verilog-style numbers <size>'<radix>value
parseNumber :: Parser Number
parseNumber = do
  size <- option "32" (many1 digit)
  char '\''
  radix <- option 'd' $ oneOf "hH" <|> oneOf "dD" <|> oneOf "oO" <|> oneOf "bB"
  let (thisDigit, convert_digits) =
        case toLower radix of
          'h' -> (hexDigit, hexToInteger)
          'o' -> (octDigit, octToInteger)
          'd' -> (digit   , read)
          'b' -> (binDigit, binToInteger)
          _  ->  error "parseNumber: Internal error"
  val <- many1 thisDigit
  let size' = read size :: Integer
      val' = convert_digits val :: Integer
      num = makeNumber val' size'
    in
    either fail return num

main :: IO ()
main = do
  args <- getArgs
  let results = map (parse parseNumber "parseNumber") args
  mapM_ print results

Note: In true Verilog numbers have the format <size>'<signed><radix>value but in my application negative numbers are not allowed, hence the omission of <signed>.

Let's start with your imports. You should get rid of those you don't need, e.g. System.IO, import Data.Either.Extra (fromRight). Next, you shouldn't mix integral and floating point calculations, unless you're fine with imprecise results. For example, integerLog will give the wrong answer around 1024:

ghci> integerLog (2^1023)
1023
ghci> integerLog (2^1024)
179769313486231590772930519078902473361797697894230657273430081157732675805500963132708477322407536021120113879871393357658789768814416622492847430639474124377767893424865485276302219601246094119453082952085005768838150682342462881473913110540827237163350510684586298239947245938479716304835356329624224137216

Next, record field names are functions:

data Number = Number {val::Integer, len:: BitSize} deriving Show

This gives you two functions,

val :: Number -> Integer
len :: Number -> Integer

Unless you're fine with those functions at global namespace, it's usually a better idea to either a) use a complete name, e.g. value, bitLength, or b) prefix/suffix them with the type's name, e.g. numberValue or lenNumber. The story slightly changes if you want to use lenses and Template Haskell, but that's for another post.

What follows afterwards are several top-level functions without type signature, which is considered bad style:

readBin = readInt 2 (`elem` "01") digitToInt

-- conversions String -> Number
hexToInteger x = fst $ readHex x !! 0 :: Integer
octToInteger x = fst $ readOct x !! 0 :: Integer
binToInteger x = fst $ readBin x !! 0 :: Integer

All those functions are partial, e.g.

ghci> hexToInteger "Hello"
*** Exception: Prelude.!!: index too large

Your use prevents this kind of error, since you're using ithexToInteger only on valid combinations, but you shouldn't export them either. That's not really an issue here since you've wrote a Main (main) module, but you should keep that in mind.

However, those functions would be a lot nicer with the appropriate type:

convertWith :: ReadS Integer -> String -> Maybe Integer
convertWith fn xs = case fn xs of
                      [(n,"")] -> Just n
                      _        -> Nothing

hexToInteger, octToInteger, binToInteger :: String -> Maybe Integer
hexToInteger = convertWith readHex
octToInteger = convertWith readOct
binToInteger = convertWith (readInt 2 (`elem` "01") digitToInt)

In this case we've dumped the plumbing into convertWith, added type signatures and made the functions non-partial. Now, let's step back to your smart-constructor:

-- |makeNumber is a safe constructor of an Either String Number
makeNumber :: Integer -> Integer -> (Either String Number)
makeNumber v l
  | v < 0     = Left "makeNumber: Negative values not allowed in Number"
  | size > l  = Left $ "makeNumber: Value " ++ show v ++ " is "
    ++ show size ++ " bits long, should be at most " ++ show l ++ " bits"
  | size <= 0 = Left $ "makeNumber: Size can't be negative or zero"
  | otherwise = Right $ Number v l
  where size = numberSize v

While that's better than just a Maybe Number, it's still not optimal if you have an application that's creating those numbers. Since sum types aren't expensive to write, let's use one:

data NumberError = ENegative
                 | ENegativeSize
                 | EBitOverflow Integer Integer
 deriving (Show, Eq)

That way you can separate the formatting from the number handling:

makeNumber :: Integer -> BitSize -> (Either NumberError Number)
makeNumber v l
  | v < 0     = Left $ ENegative
  | size > l  = Left $ EBitOverflow v l
  | size <= 0 = Left $ ENegativeSize
  | otherwise = Right $ Number v l
  where size = numberSize v

You could also provide some functions, e.g.

eNegative    :: Either NumberError a
eBitOverflow :: Integer -> BitSize -> Either NumberError a

to ease the use of those, but that's an overkill here.

Speaking of overkills, did you notice the BitSize above? Many of your functions use Integer -> Integer -> …. But what do those Integer's mean? Which one is the value? Which one is the bitsize? And is Integer as bitsize really a good idea?

Therefore, try to add some documentation at the type level, even if it isn't a new type:

type BitSize = Int

Now, let's have a look at the parser:

-- |parse Verilog-style numbers <size>'<radix>value
parseNumber :: Parser Number
parseNumber = do
  size <- option "32" (many1 digit)
  char '\''
  radix <- option 'd' $ oneOf "hH" <|> oneOf "dD" <|> oneOf "oO" <|> oneOf "bB"
  let (thisDigit, convert_digits) =
        case toLower radix of
          'h' -> (hexDigit, hexToInteger)
          'o' -> (octDigit, octToInteger)
          'd' -> (digit   , read)
          'b' -> (binDigit, binToInteger)
          _  ->  error "parseNumber: Internal error"
  val <- many1 thisDigit
  let size' = read size :: Integer
      val' = convert_digits val :: Integer
      num = makeNumber val' size'
    in
    either fail return num

This is well done, except for two minor quirks: you can collect all oneOf <prefix> parsers into a single one:

oneOf "hHdDoObB"

and your use of error, which makes this parser partial again. Instead, you could have used

  (thisDigit, convert_digits) <-
        case toLower radix of
          'h' -> return (hexDigit, hexToInteger)
          'o' -> return (octDigit, octToInteger)
          'd' -> return (digit   , read)
          'b' -> return (binDigit, binToInteger)
          _  ->  fail "parseNumber: Internal error"

Note that parseNumber slightly changes if you use String -> Maybe Integer from above (you should use readMaybe instead of read in this case). Also note that parseNumber isn't a good name in this case, it indicates that you're parsing, but you only define a parser. numberParser is usually slightly better.

Also, make sure that your variables follow the same naming convention. convert_digits kind of falls of the rest of your names.

Last but not least, it's strange that you get your numbers as arguments, e.g.

$ ./MyApplication 32\'b100101001

Usually, you take those values from stdin and print them to stdout. The interact function will come in handy.

Bonus content: Separated concerns and an applicative Parser

So, now that we had a look at your code, how would I write it? I would probably introduce a function that takes the bitsize, the prefix, and the rest:

type BitSize      = Word
data NumberFormat = Bin | Oct | Dec | Hex

makeNumber :: BitSize -> NumberFormat -> String -> Either NumberError Number
makeNumber l d xs = do
  num <- readWith d xs
  case num of
   _ | num  < 0 -> eNegative
   _ | bits > l -> eBitOverflow num l
   _            -> return $ Number num l
  where num  = readWith d xs
        bits = fromIntegral $ numberSize num

readWith :: NumberFormat -> String -> Either NumberError Number
readWith d xs = maybeToEither $ case d of
                   Bin -> binToInteger xs
                   Oct -> octToInteger xs
                   Dec -> readMaybe    xs
                   Hex -> hexToInteger xs

Essentially that's still your code, only put into two functions and using the NumberError above. This makes it possible to later test the parts of your previously large parseNumber. Note that all those functions are rather small, and do only one thing (and one thing well).

Now what's left is the Parser. Instead of the previous Monad one, we can now easily use an Applicative one. For this, we provide three additional parsers that parse the sub-expressions <bits>'<format><digits>:

bitSizeParser :: Parser BitSize
bitSizeParser = option 32 $ fmap read $ many1 digits
-- read is fine here, since we use only digits

numberFormatParser :: Parser NumberFormat
numberFormatParser = option Dec $ fmap toFormat $ oneOf "hHoOdDbB"
  where toFormat c = case toLower c of
          'h' -> Hex
          'b' -> Bin
          'd' -> Dec
          'o' -> Oct

digitsParser :: Parser String
digitsParser = many1 $ oneOf "0123456789abcdef"

Now what's left is using those three parsers together and feeding them into makeNumber:

 import Control.Applicative ((<$>), (<*>)) -- not needed in GHC 7.10 or higher

numberParser' :: Parser (Either NumberError Number)
numberParser' =
  makeNumber <$> bitSizeParser <*> numberFormatParser <*> digitsParser

numberParser :: Parser Number
numberParser = numberParser' >>= either fail return

Note that the parts of our parser are now reasonably dumb. They only get things, i.e. <bits>'<format><digits>. The logic is now in makeNumber, which can be used without actually parsing, which might be handy if you want to check values from a database or similar.