Method to delegate command to other methods Since I am working alone on this, I would like someone who would take a look and point out any mistakes or things I could have done better.
My questions:

How could I improve this big switch statement?
Is there any obvious beginner flaws?

The executeModeration is a method I call when a command is an moderation command. The command.parameters[0] contains the parameters a commmand needs to be executed
const executeModeration = async (command, channel) => {
  switch (command.command)  {
    case "ban":
      if(command.parameters[0])
        await moderation.ban(channel, command.parameters[0], command.parameters[1] !== undefined ? command.parameters[1] : '');
      break;
    case "unban":
      if(command.parameters[0])
        await moderation.unban(channel, command.parameters[0]);
      break;
    case "timeout":
      if(command.parameters[0])
        await moderation.timeout(channel, command.parameters[0], command.parameters[1] !== undefined ? command.parameters[1] : 300, command.parameters[2] !== undefined ? command.parameters[2] : '');
      break;
    case "emoteonly":
      await moderation.emoteonly(channel);
      break;
    case "emoteonlyoff":
      await moderation.emoteonlyoff(channel);
      break;
    case "followersonly":
      await moderation.followersonly(channel,command.parameters[0] !== undefined ? command.parameters[0] : 30);
      break;
    case "followersonlyoff":
      await moderation.followersonlyoff(channel);
      break;
    case "r9kbeta":
      await moderation.r9kbeta(channel);
      break;
    case "r9kbetaoff":
      await moderation.r9kbetaoff(channel);
      break;
    case "slow":
      await moderation.slow(channel, command.parameters[0] !== undefined ? command.parameters[0] : 30);
      break;
    case "slowoff":
      await moderation.slowoff(channel);
      break;
    case "subscribers":
      await moderation.subscribers(channel);
      break;
    case  "subscribersoff":
      await moderation.subscribersoff(channel);
      break;
    case  "mod":
      if(command.parameters[0])
        await moderation.mod(channel, command.parameters[0]);
      break;
    case  "unmod":
      if(command.parameters[0])
        await moderation.unmod(channel, command.parameters[0]);
      break;
    case  "vip":
      if(command.parameters[0])
        await moderation.vip(channel, command.parameters[0]);
      break;
    case  "unvip":
      if(command.parameters[0])
        await moderation.unvip(channel, command.parameters[0]);
      break;
    case  "clear":
      await moderation.clear(channel);
      break;
    case  "host":
      if(command.parameters[0])
        await moderation.host(channel, command.parameters[0]);
      break;
    case  "unhost":
      await moderation.unhost(channel);
      break;
    case  "commercial":
      if(command.parameters[0])
        await moderation.commercial(channel, command.parameters[0]);
      break;
  }

Spread Similar to Mohammed's answer, but without mutation and with spread syntax, which I find easier to read than .apply, you can do something like:
const executeModeration = async (command, channel) => {
    await moderation[command.command](...command.parameters);
}

Default parameters
You use a number of conditional operators when calling functions, eg:
.ban(
    channel,
    command.parameters[0],
    command.parameters[1] !== undefined ? command.parameters[1] : ''
);

.timeout(
    channel,
    command.parameters[0],
    command.parameters[1] !== undefined ? command.parameters[1] : 300,
    command.parameters[2] !== undefined ? command.parameters[2] : ''
);

If possible, it would be good to define the methods such that they assign default parameters, rather than having to pass the defaults from up above. For example, for the timeout method, you could define it like:
timeout(arg1, arg2 = 300, arg3 = '') {
}

and then simply pass the spread parameters when calling timeout from the script in your question, and the default parameters will take care of the rest.
If you cannot modify the moderation object's methods, then a DRY way to create these default parameters would be to make an object indexed by command, whose values are functions which need default parameters and call the moderation method. For example, for .ban and .timeout:
const executeModeration = ({ command, parameters }, channel) => {
    const fnsWithDefaultParams = {
        ban(arg1, arg2 = '') {
            return moderation.ban(channel, arg1, arg2);
        },
        timeout(arg1, arg2 = 300, arg3 = '') {
            return moderation.timeout(channel, arg1, arg2, arg3);
        },
        // ...
    };
    return fnsWithDefaultParams[command]
        ? fnsWithDefaultParams[command](...parameters)
        : moderation[command](...parameters)
};

Note that there's now no need for an async function if the function explicitly returns a Promise, like above.


Tests for a POJO class ProgressableItem I have tests for simple POJO class ProgressableItem (i.e. the one which has attributes total work and work done).

I use JUnit 3 (it's from Android application).

public class ProgressableItemTest extends AndroidTestCase {
    private ProgressableItem item;

    @Override
    protected void setUp() throws Exception {
        super.setUp();
        item = new ProgressableItem("item", "test item", 0, 100);
    }

    public void testNotEquals() {
        ProgressableItem itemWithDifferentName = copyOf(item);
        itemWithDifferentName.setName("Not the same " + item.getName());
        assertThat(itemWithDifferentName, not(item));
        ProgressableItem itemWithDifferentDescription = copyOf(item);
        itemWithDifferentName.setDescription("Not the same " + item.getDescription());
        assertThat(itemWithDifferentDescription, not(item));
        ProgressableItem itemWithDifferentProgress = copyOf(item);
        itemWithDifferentName.setProgress(item.getProgress() + 1);
        assertThat(itemWithDifferentProgress, not(item));
        ProgressableItem itemWithDifferentMax = copyOf(item);
        itemWithDifferentName.setMax(item.getMax() + 1);
        assertThat(itemWithDifferentMax, not(item));
    }

    private static ProgressableItem copyOf(ProgressableItem item) {
        return new ProgressableItem(item.getName(), item.getDescription(), item.getProgress(), item.getMax());
    }

}

public interface Handler<T> {
    void handle(T obj);
}

Then I modified it into:

public void testNotEquals() {
    for (Handler<ProgressableItem> progressableItemModifier : PROGRESSABLE_ITEM_MODIFIERS) {
        ProgressableItem differentItem = copyOf(item);
        progressableItemModifier.handle(differentItem);
        assertThat(differentItem, not(item));
    }
}

private static final Collection<? extends Handler<ProgressableItem>>
        PROGRESSABLE_ITEM_MODIFIERS = Arrays.asList(
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setName("Not the same " + obj.getName());
                    }
                },
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setDescription("Not the same " + obj.getDescription());
                    }
                },
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setProgress(obj.getProgress() + 1);
                    }
                },
                new Handler<ProgressableItem>() {
                    @Override
                    public void handle(ProgressableItem obj) {
                        obj.setMax(obj.getMax() + 1);
                    }
                }
);

But now I have many more lines of code.

Which of the two options is better? Should I separate the test method into 4 small methods? Is there no need to check each case?

A couple of tips for unit testing:

One test case per subject: so you should definitely split the cases
Ideally no logic at all: avoid loops
Duplication is acceptable: clarity is most important

I would write this way:

public void testNotEquals_IfNameChanged() {
    ProgressableItem itemWithDifferentName = copyOf(item);
    itemWithDifferentName.setName("Not the same " + item.getName());
    assertThat(itemWithDifferentName, not(item));
}

public void testNotEquals_IfDescriptionChanged() {
    ProgressableItem itemWithDifferentDescription = copyOf(item);
    itemWithDifferentName.setDescription("Not the same " + item.getDescription());
    assertThat(itemWithDifferentDescription, not(item));
}

public void testNotEquals_IfProgressChanged() {
    ProgressableItem itemWithDifferentProgress = copyOf(item);
    itemWithDifferentName.setProgress(item.getProgress() + 1);
    assertThat(itemWithDifferentProgress, not(item));
}

public void testNotEquals_IfMaxChanged() {
    ProgressableItem itemWithDifferentMax = copyOf(item);
    itemWithDifferentName.setMax(item.getMax() + 1);
    assertThat(itemWithDifferentMax, not(item));
}

Since you never change item, you don't really need the setUp method:

private final ProgressableItem item = new ProgressableItem("item", "test item", 0, 100);

Of course just because it's final nothing prevents you from mutating it.

To be extra safe and stay efficient, you could create an unmodifiable ProgressableItem, for example:

private final ProgressableItem item = new ProgressableItem("item", "test item", 0, 100) {
    @Override
    public void setName(String name) {
        throw new UnsupportedOperationException();
    }
    // ... + the other setters too
};


Exporting objects in various formats while reporting progress Description
A WinForms application has a function to export objects of the following type, in various formats:
class Item
{
    public int id { get; set; }
    public string description { get; set; }
}

At the click of a button in a window, a SaveFileDialog is shown, and currently it provides the option to save the data in .txt, .csv or .xlsx format. Since there are sometimes hundreds or thousands of objects, and the UI should not freeze up, a Task is used to run this operation. This implementation works, but could be improved.
Code
public partial class ExportWindow : Form
{
    // objects to be exported
    List<Item> items;

    // event handler for the "Export" button click
    private async void exportButton_click(object sender, System.EventArgs e)
    {
        SaveFileDialog exportDialog = new SaveFileDialog();
        exportDialog.Filter = "Text File (*.txt)|*.txt|Comma-separated values file (*.csv)|*.csv|Excel spreadsheet (*.xlsx)|*.xlsx";
        exportDialog.CheckPathExists = true;
        DialogResult result = exportDialog.ShowDialog();
        if (result == DialogResult.OK)
        {
            var ext = System.IO.Path.GetExtension(saveExportFileDlg.FileName);

            try
            {
                // update status bar
                // (it is a custom control)
                statusBar.text("Exporting");

                // now export it
                await Task.Run(() =>
                {
                    switch (ext.ToLower())
                    {
                        case ".txt":
                            saveAsTxt(exportDialog.FileName);
                            break;

                        case ".csv":
                            saveAsCsv(exportDialog.FileName);
                            break;

                        case ".xlsx":
                            saveAsExcel(exportDialog.FileName);
                            break;

                        default:
                            // shouldn't happen
                            throw new Exception("Specified export format not supported.");
                    }
                });
            }
            catch (System.IO.IOException ex)
            {
                 statusBar.text("Export failed");
                 logger.logError("Export failed" + ex.Message + "\n" + ex.StackTrace);

                 return;
            }
        }
    }

    private delegate void updateProgressDelegate(int percentage);

    public void updateProgress(int percentage)
    {
        if (statusBar.InvokeRequired)
        {
            var d = updateProgressDelegate(updateProgress);
            statusBar.Invoke(d, percentage);
        }
        else
        {
            _updateProgress(percentage);
        }
    }

    private void saveAsTxt(string filename)
    {
        IProgress<int> progress = new Progress<int>(updateProgress);

        // save the text file, while reporting progress....
    }

    private void saveAsCsv(string filename)
    {
        IProgress<int> progress = new Progress<int>(updateProgress);

        using (StreamWriter writer = StreamWriter(filename))
        {
            // write the headers and the data, while reporting progres...
        }
    }

    private void saveAsExcel(string filename)
    {
        IProgress<int> progress = Progress<int>(updateProgress);

        // EPPlus magic to write the data, while reporting progress...
    }
}

Questions
How can this be refactored to make it more extensible? That is, if I wanted to add support for more file types, make it easy and quicker to modify. The switch statement could get very long. Essentially, how to comply with the Open/Closed principle?

I would suggest moving the actual export(s) into their own class.  We can create an interface for exports.  Something along the lines of
public interface IExport<T>
{
    Task SaveAsync(string fileName, IEnumerable<T> items, IProgress<int> progress = null);
    string ExportType { get; }
}

Then each export type can implement this interface.
public class ExportItemsToText : IExport<Item>
{
    public Task SaveAsync(string fileName, IEnumerable<Item> items, IProgress<int> progress = null)
    {
        throw new NotImplementedException();
    }

    public string ExportType => "txt";
}

Then in your constructor of ExportWindow
public ExportWindow(IEnumerable<IExport<Item>> exports)
{
    // if using DI otherwise could just fill in dictionary here
    ExportStrategy = exports.ToDictionary(x => x.ExportType, x => x);
}

Instead of a switch statement you can now just look up the key in the dictionary to find what export should be ran and if not found would be the same as your default case.
IExport<Item> exporter;
if (ExportStrategy.TryGetValue(ext.ToLower(), out exporter))
{
    await exporter.SaveAsync(exportDialog.FileName, items, new Progress<int>(updateProgress))
}
else
{
    throw new Exception("Specified export format not supported.");
}

Now in the future if adding support for more types you just implement the interface and update your DI container.  Or if not using DI then would need to add it to the constructor of your ExportWindow.
I don't think this is a great idea but If you really don't want to create a class per export, which I think you should, you could make the dictionary IDictionary<string, Action<string>> then just put your methods in there and when adding a new type create the method and update the dictionary.


Optimizing SPOJ edit distance solution I was doing a standard problem of DP (dynamic programming) on SPOJ  Edit Distance using Python.

t = raw_input()

for i in range(int(t)):
   a,b = raw_input(),raw_input()

   r = len(a)
   c = len(b)
   x = [[0]*(c+1) for j in range(r+1)]

   for j in range(c+1):
       x[0][j] = j

   for j in range(r+1):
       x[j][0] = j

   for j in range(1,r+1):
       for k in range(1,c+1):
           if(b[k-1]!=a[j-1]):
              x[j][k] = min(x[j-1][k-1]+1,x[j-1][k]+1,x[j][k-1]+1)
           else:
              x[j][k] = min(x[j-1][k-1],x[j-1][k]+1,x[j][k-1]+1)

   print x[r][c]

The solution I have proposed is giving T.L.E (Time Limit Exceeded) even though I am using D.P. Is there any way to optimize it further in terms of time complexity or with respect to any feature of Python 2.7 such as input and output?

I've solved that problem in java and c++ (2nd and 3th places in best solutions category :) so I can compare the local and the remote execution time in order to see - how much faster should be your solution in order to pass.

So, the local execution time of my java solution is 78 ms for the such testcase (10 pairs x 2000 chars), the robot's time is 500 ms, so my PC is ~6.5 times faster. Then the following python DP solution takes 3.6 seconds on my PC so, it would take ~23.5 seconds on the remote PC. So if the remote time limit 15 seconds, the following solution must be minimum ~ 1.56 times faster in order to pass. Ufff....

import time

try:
    # just to see the Python 2.5 + psyco speed - 17 times faster than Python 2.7 !!!
    import psyco
    psyco.full()
except:
    pass

def editDistance(s1, s2):
    if s1 == s2: return 0
    if not len(s1):
       return len(s2)
    if not len(s2):
       return len(s1)
    if len(s1) > len(s2):
        s1, s2 = s2, s1
    r1 = range(len(s2) + 1)
    r2 = [0] * len(r1)
    i = 0
    for c1 in s1:
        r2[0] = i + 1
        j = 0
        for c2 in s2:
            if c1 == c2:
                r2[j+1] = r1[j]
            else:
                a1 = r2[j]
                a2 = r1[j]
                a3 = r1[j+1]
                if a1 > a2:
                    if a2 > a3:
                        r2[j+1] = 1 + a3
                    else:
                        r2[j+1] = 1 + a2
                else:
                    if a1 > a3:
                        r2[j+1] = 1 + a3
                    else:
                        r2[j+1] = 1 + a1
            j += 1
        aux = r1; r1 = r2; r2 = aux
        i += 1
    return r1[-1]

if __name__ == "__main__":
    st = time.time()
    t = raw_input()
    for i in range(int(t)):
        a, b = raw_input(), raw_input()
        print editDistance(a, b)
    #print "Time (s): ", time.time()-st

What I can say - It's very very hard or may be impossible to pass that puzzle using Python and DP approach (using java or c++ - peace of cake). Wait, wait - ask you - what about that two guys that passed using python? Ok, the answer is easy - they use something different. What's exactly? Something that I've used for java solution I think. That stuff just blows away the competitors....


Actionscript web service and web method call classes I'm trying to isolate a webservice in its own class, and I plan to add separate classes to each webmethod there is in the webservice.  What I have so far works, but I have this feeling tickling that I've missed something (except for the invisible variable declarations down here, I didn't want to clog the page).

Webservice instantiation class and its fault handler:

 public class CfdWS
        {

            [Bindable]
            private var model:ModelLocator = ModelLocator.getInstance();

            public function loadWebService():WebService{
                var webService : WebService = new WebService();
                webService.wsdl = model.configXML.cfdwsWSDL;
                webService.addEventListener(FaultEvent.FAULT, onWebServiceFault);
                webService.loadWSDL();
                return webService;
            }

            private function onWebServiceFault(event:FaultEvent):void{
                var fault: Fault = event.fault;
                var message:String = "\ncodigo: " + fault.faultCode;
                message += "\nDetalle: " + fault.faultDetail;
                Alert.show("Error de webservice:" + message);
            }
        }
    }

The following is my webservice method call class. I have written only what I think is the essential code for the question.

public class GeneratePDF extends CfdWS{

    public function generatePDF():void{
                webService = loadWebService();
                webService.addEventListener(LoadEvent.LOAD, doGeneratePDF);
            }
            private function doGeneratePDF(event:LoadEvent):void{
                webService.generatePDF.addEventListener(ResultEvent.RESULT, generatePDFResultHandler);
                webService.generatePDF(pdfData);
            }
            private function generatePDFResultHandler (event:ResultEvent):void{
                // After getting what I want, I remove the event listeners here.
            }
}

I'm trying to re-write an application that is already in production while on testing phase (testing for the next version I mean).

I don't see why would you put every method of the service in a separate class. A "method" is a function of the class. I imagine you wanted to decouple your code, but doing it this way you will force a lot of overhead:

the service is instantiated for every 'method' called, and then, hopefully, garbage collected (as you remove event listeners and there's no more references to the service left)
because of above, the service is stateless; with time you may want to add some functionality like caching, but you'd need to change whole code structure for that
CfdWS - not descriptive name; your way of decoupling code will force you to make three or even five times more classes then you normally would, so I would expect a hell on the file-naming level
really, dividing to so many classes is not a good idea - you don't want to switch between files all the time; try to put related code in one Class, and if it grows big, create some helper classes

I think you already understand the benefit of a good MVC implementation, try Robotlegs, it really makes a life easier:
http://www.robotlegs.org/


A novice student record system I am an extreme novice in Java and thus trying to practise some online exercises. One of them involved creating a basic student record system.
This is what I've come up with but I think a lot of things are quite redundant in my code so any reviews are greatly appreciated.

Student.java

class Student {

    int rollno;
    String name;

    Student(int r, String n) {
        this.rollno = r;
        this.name = n;
    }
}

StudentDB.java

import java.util.Scanner;

class Main {

    public static void getStudents(Student ...students) {
        int i = 1;
        for (Student x : students) {
            System.out.println("Student Number : " + i);
            System.out.println("Name -> "+ x.name);
            System.out.println("Roll Number -> " + x.rollno + "\n");
            i++;
        }
    }

    public static void main(String[] args) {

        Scanner sc = new Scanner(System.in);

        System.out.print("Number of students? ");
        int num = sc.nextInt();

        String[] name = new String[num];
        int[] rollno = new int[num];

        System.out.println("Enter those " + num + " students one by one");

        for (int i = 0; i < num; i++) {
            System.out.println("Enter details for student no. " + (i + 1));
            System.out.print("Enter Name: ");
            name[i] = sc.next();
            System.out.print("Enter Roll Number: ");
            rollno[i] = sc.nextInt();
            System.out.println("\n");
        }

        Student[] students = new Student[num];

        for (int i = 0; i < num; ++i) {
            students[i] = new Student(rollno[i], name[i]);
        }

        getStudents(students);
    }
}

I would really love it if someone could tell me as to how can I pass those name and rollno arrays directly to the vararg function getStudents without creating that Student[] object array.

Main:

import java.util.Scanner;

class Main {

    public static void main(String[] args) {

        Scanner scanner = new Scanner(System.in);
        Student[] students = StudentRoll.gatherStudents(scanner);
        StudentRoll.printStudents(students);

    }
}

Student:

class Student {

    private int rollNumber;
    private String name;

    Student(int rollNumber, String name) {
        this.rollNumber = rollNumber;
        this.name = name;
    }

    int getRollNumber() {
        return rollNumber;
    }
    void setRollNumber(int rollNumber) {
        this.rollNumber = rollNumber;
    }

    String getName() {
        return name;
    }
    void setName(String name) {
        this.name = name;
    }
}

StudentRoll:

class StudentRoll {

    static Student[] gatherStudents(Scanner scanner){

        System.out.print("Number of students? ");
        int numberOfStudents = scanner.nextInt();

        System.out.println("Enter those " + numberOfStudents + " students one by one");
        Student[] students = new Student[numberOfStudents];

        for (int index = 0; index < numberOfStudents; index++) {
            System.out.println("Enter details for the next student no. ");
            System.out.print("Enter Name: ");
            String studentName = scanner.next();
            System.out.print("Enter Roll Number: ");
            int rollNumber = scanner.nextInt();
            students[index] = new Student(rollNumber, studentName);
            System.out.println("\n");
        }
        return students;
    }

    static void printStudents(Student[] students){
        int i = 1;
        for (Student x : students) {
            System.out.println("Student Number : " + i);
            System.out.println("Name -> " + x.getName());
            System.out.println("Roll Number -> " + x.getRollNumber() + "\n");
            i++;
        }
    }
}

I can't post many notes right now, but will as soon as I can. Think the main thing here is that your code isn't very object-oriented, but Java is an object-oriented language. It's also generally a good idea to name variables something other than num, so that when you're working on a method that has 5 different int variables, you aren't tempted to call them num1 , num2, etc., but what they actually represent. I'm also not a big fan of using a for-in when you reference an index, but it didn't seem like the worst thing in the world so I left it in there.


JS composition - Components depend on others NOTE: I am not familiar with a lot of terminology. If I am naming things incorrectly, please correct me!

Making small game engine for learning. Mostly using inheritance, but the more my project grows, the more I feel like objects are inheriting to much.

Example of inheriting classes:

Body // Base for x/y and width/height
  ↪Moveable
    ↪Drawable
      ↪Collidable

Not very clean. Parents are interchangeable with their children. Basically using inheritance primarily to organize my code and dump all the functionality into a big monster class.

Sometimes, inheriting everything makes sense, but not always. For example:

A "Player" would inherit everything. It can move, it needs to be drawn
and it collides with other objects in the game
A "Tree" would only need "Body", "Drawable" and "Collidable" since it does      not move
An "Area" would need "Body" and "Collidable" since it is invisible, static and only needs to know if something comes within its range

After researching, composition seems to be a good solution. Saw examples, but not exactly like the one I opted for where instances require on other instances from the parent. Child instances also have references to their parent.

function Tree(x, y){
    this.body = new Body(this, x, y, 100, 100);
    this.draw = new Drawable(this);
}

function Body(parent, x, y, width, height){
    this.parent = parent;

    this.x = x;
    this.y = y;

    this.width = width;
    this.height = height;

    this.getPosition = function(){ /* Returns x and y */ };
    this.getDimensions = function(){ /* Returns width and height */ };
}

function Drawable(parent){
    this.parent = parent;

    this.image = function(image){
        var position = this.parent.body.getPosition();
        var dimensions = this.parent.body.getDimensions();
        // Use image, position and dimensions to draw an image on a canvas
    }
}

Now if I wanted to draw the tree, I would do something like

tree = new Tree(500, 500);
tree.draw.image('tree.jpg');

I'm not sure if I am abusing the fact that JS objects are passed by reference to create my objects. The main drawback I see if that you need to know what requires what, but that doesn't seem hard to overcome. For example, "Drawable" needs the parent to have an instance of "Body" to work.

Is there something I'm missing or is this a good solution to implement?

In a case like this I would use mixins (though not everyone agrees when it comes to mixins vs composition). I would write this something like:

let BodyMixin = {
    getPosition:   function(){ /* Returns x and y */ };
    getDimensions: function(){ /* Returns width and height */ };
}

let DrawableMixin = {
   image: function(image) {
        var position   = this.getPosition();
        var dimensions = this.getDimensions();
        // Use image, position and dimensions to draw an image on a canvas
   }
}

function Tree(x, y){
}
Object.assign(Tree.prototype, BodyMixin, DrawableMixin);

Then your call would just be:

tree.image('tree.jpg');

Note: if you are using TypeScript or something similar there are other ways to implement mixins.

-- Update 1:

It might also be worth doing something like:

let BodyMixin = {
    initializeBody:      function(x,y,w,h) { ... };
    ...
}

function Tree(x, y){
    this.initializeBody(x, y, 100, 100);
}
Object.assign(Tree.prototype, BodyMixin, DrawableMixin);


Background image loader AsyncTask Usually, AsyncTasks are meant to be one-shot, i.e. you start it, it fetches some data, displays the result and dies. However, I'm now using a long-running AsyncTask to load images as the need arises. So far it's working great, but I'd just like to know if this is good practice & if there's any threading detail that I missed. Here's my slimmed down implementation:

public class ImageLoader extends AsyncTask<Void, Object, Void> {
    private final List<String> pendingURLs = new Vector<String>();
    private Thread backgroundThread;

    @Override
    protected Void doInBackground(Void... params) {
        this.backgroundThread = Thread.currentThread();
        while (!backgroundThread.isInterrupted()) {
            String url = null;
            synchronized(this) {
                if (pendingURLs.size() > 0) {
                    url = pendingURLs.remove(0);
                }
            }
            if (url != null) {
                // fetch image...
                publishProgress(url, fetchedBitmap); // in onProgressUpdate I call a listener that can be implemented by any Fragment or other component
            }
            try {
                synchronized(this) {
                    if (pendingURLs.size() == 0) {
                        wait();
                    }
                }
            } catch (InterruptedException e) {
                break;
            }
        }
    }

    public syncrhonized void addUrl(String url) {
        pendingURLs.add(url);
        notify();
    }

    public Thread getBackgroundThread() {
        return backgroundThread;
    }

}

Then, in my Fragment#onCreate() I create a new instance and execute it like this:

mLoader = new ImageLoader();
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB) {
    mLoader.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
} else {
    mLoader.execute();
}

Finally, in my Fragment#onDestroy() I do:

if (mLoader != null && mLoader.getBackgroundThread() != null) {
    mLoader.getBackgroundThread().interrupt(); // this will cause it to break out of the while loop and finish the thread
}

For starters, you should replace your Vector with an ArrayBlockingQueue .  Then you can drop the wait and the null check.  Can also then remove synchronization on the add method since its handled by the data structure.

private final BlockingQueue<String> pendingURLs = new ArrayBlockingQueue<String>(64);
protected void doInBackground(Void... params) {
    this.backgroundThread = Thread.currentThread();
    while (!backgroundThread.isInterrupted()) {
        String url = pendingURLs.take();
        // fetch image...
        publishProgress(url, fetchedBitmap); // in onProgressUpdate I call a listener that can be implemented by any Fragment or other component
    }

public void addUrl(String url) {
    pendingURLs.put(url);
}

You should probably replace it with just a new thread instead of using asynctask at all.

public class ImageLoader implements Runnable {
  private final BlockingQueue<String> pendingURLs = new ArrayBlockingQueue<String>();

  public void run() {
    while (true) {
        String url = pendingURLs.take();
        // fetch image...
        publishProgress(url, fetchedBitmap); // in onProgressUpdate I call a listener that can be implemented by any Fragment or other component
    }
  }

  public void addUrl(String url) {
    pendingURLs.put(url);
  }
}

Then you can create it in your initialization somewhere, ie

 ImageLoader loader = new ImageLoader();
 new Thread(loader, "BackgroundLoader").start();


Count the numbers without consecutive repeated digits Count the numbers without consecutive repeated digits

Asha loves mathematics a lot. She always spends her time by playing with digits. Suddenly, she is stuck with a hard problem, listed below. Help Asha to solve this magical problem of mathematics.

Given a number N (using any standard input method), compute how many integers between 1 and N, inclusive, do not contain two consecutive identical digits. For example 1223 should not be counted, but 121 should be counted.

Test Cases

7 => 7
1000 => 819
3456 => 2562
10000 => 7380

The code snippet that i submitted  for the above challenge was given below

f(int n)
{
    int i=1;
    int c=n;

    for(; i<=n; i++)
    {
        int m=-1;
        int x=i;

        while(x)
        {
            if (x % 10 == m)
            {
                c--;
                break;
            }
            m = x % 10;
            x /= 10;
       }
   }
   return c;
}

When I submitted the above code in the competition a few days ago I ranked the First. but yesterday I was at no 10-11-13 and now I am ranked 13 right now.

What was my mistake? What should I improve so that my rank will be the first in competitive programming?

The problem statement gives a great hint: mathematics. So do not brute force.

Fix the leading digit less than the leading digit of \$N\$ (including zero). For the next digit you have 9 choices (to avoid repetition). For the next digit you again have 9  choices, etc. The is, any leading digit less than the leading digit of \$N\$ gives \$9^k\$ "good" numbers.

With a leading digit equals to leading digit of \$N\$ you have less choices, because you have to stay below \$N\$. The best way to deal with is is to remove the leading digit of \$N\$, and apply the same logic to the remaining number.

For example, for \$N=3456\$ it works like this \$3 \times 9^3 + 4 \times 9^2 + 5 \times 9 + 6 = 2562\$

Now convert it to the code.

Hint: you can also go right to left.


Check if string can be rearranged into a palindrome I created a function to test whether a single word string can be a rearranged into a palindrome (doesn't have to be a real word). My logic was if the string has an even number of letters, then each letter must occur twice. If the string has an odd number of letters, each letter must occur twice except for one.

function checkPalindrome(str) {
    var letters = str.split(''),
        unique = [],
        match = {},
        single = [];

    var findUnique = function(arr, letter) {
        for (var k = 0; k < arr.length; k++) {
            if (arr[k].match(letter) && !unique.join('').match(letter)) {
                unique.push(arr[k]);
            }
        }
    }

    var sortLetters = function(arr, letter) {
        for (var i = 0; i < arr.length; i++) {
            if (arr[i] === letter) {
                if (match[letter] === undefined) {
                    match[letter] = 1;
                } else {
                    match[letter]++;
                }
            }
        }
    }

    for (var j = 0; j < letters.length; j++) {
        findUnique(letters, letters[j]);
    }

    for (var m = 0; m < unique.length; m++) {
        sortLetters(letters, unique[m]);
    }

    for (var key in match) {
        if (match[key] % 2 !== 0) {
            single.push(match[key])
        }
    }

    if (letters.length % 2 === 0) {
        if (single.length === 0) {
            return true;
        } else {
            return false;
        }
    } else {
        if (single.length === 1) {
            return true;
        } else {
            return false;
        }
    }
}

Any tips on how to improve this function in terms of efficiency ?

Your assumption is not quite correct.

Even number of letters: every letter must occur even amount of times (not necessarily twice)
Odd number letters: one letter must occur an odd amount of times, every other letter the same as above.

For example, aaa is a valid palindrome where no letter occurs once or twice.

As to your code, it is way too complex. If you look closely it has a complexity of \$O(n^2)\$, with n being the length of the string, because you have nested loops:
 you call findUnique() which contains a loop over the letters in a loop over the letters.

You just have to count all the letters and check if there are letters with odd counts. If there are more than one letter with an odd counts the string does not satisfy the above palindrome condition. Furthermore, since a string with an even number letters must not have a letter with an odd count it is not necessary to check whether string length is even or not:

even + odd = odd
n * even = even

The function could look like this:

function canRearrangeToPalindrome(str)
{
    var letterCounts = {};
    var letter;
    var palindromeSum = 0;
    for (var i = 0; i < str.length; i++) {
        letter = str[i];
        letterCounts[letter] = letterCounts[letter] || 0;
        letterCounts[letter]++;
    }
    for (var letterCount in letterCounts) {
        palindromeSum += letterCounts[letterCount] % 2;
    }

    return palindromeSum < 2;
}


Text-based adventure survival horror game This is the first couple lines of my new (will be) 8-bit co-op horror game. This is just the code for the first player text-based adventure storyline for the console game.  Please critique.

//Andrew Tew
/*
First EVER Co-Op Horror Game!
*/

#include <iostream>
#include <string>
using namespace std;

int main()
{
//Declared Variables
string Answer;

//Title Screen and Custom Letter Output
system ("title Until Death We Part");
system ("color 0f");
system ("mode 1000");
cout << "\t\t\t\t\t\t'------------------------------'\n";
cout << "\t\t\t\t\t\t|------------------------------|\n";
cout << "\t\t\t\t\t\t|--------The first ever--------|\n";
cout << "\t\t\t\t\t\t|----8-bit Horror Game Co-Op---|\n";
cout << "\t\t\t\t\t\t|--------Survival Game---------|\n";
cout << "\t\t\t\t\t\t|--------By: Andrew Tew--------|\n";
cout << "\t\t\t\t\t\t|\"The Backstory/Text Adventure\"|\n";
cout << "\t\t\t\t\t\t|------------------------------|\n";
cout << "\t\t\t\t\t\t'------------------------------'\n";
cout << "\n\n\t\t\t\tPlay in Full Screen Mode/Maximized Mode for the Best Experience!\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n";

/*End Start Screen

Begin Help Screen (Refrain from using Gotos)*/

system ("pause");
system ("cls");

cout << "\tHELP SCREEN:\n";
cout << "\t-----------\n\n\n";
cout << "\tAnswers:\n";
cout << "\tyes (ALWAYS LOWERCASE)\n";
cout << "\tno (ALWAYS LOWERCASE)\n\n";
cout << "\tWHO ARE YOU:\n";
cout << "\tPOSSIBLE ETHNICITIES(ALWAYS LOWERCASE):\n";
cout << "\twhite\n";
cout << "\tblack\n";
cout << "\tasian\n";
cout << "\tlatino\n\n";
cout << "\tPOSSIBLE HAIR COLORS(ALWAYS LOWERCASE):\n";
cout << "\tblond (male)\n";
cout << "\tblonde (female)\n";
cout << "\tbrown\n";
cout << "\tblack\n";
cout << "\twhite\n";
cout << "\tgray\n\n\n\n\n\n";
cout << "Cont. Next Page\n";

system ("pause");//Next Page Help Screen
system ("cls");

cout << "\n\n\tPOSSIBLE GENDERS(ALWAYS LOWERCASE):\n";
cout << "\tmale\n";
cout << "\tfemale\n";
cout << "\tit\n\n";
cout << "\tPOSSIBLE EYE COLORS(ALWAYS LOWERCASE):\n";
cout << "\tblue\n";
cout << "\tbrown\n";
cout << "\tgreen\n";
cout << "\tgray\n\n";
cout << "\tNAME LIMITS AND MINIMUMS:\n";
cout << "\tNames must contain at LEAST 3 letters.\n";
cout << "\tNames must contain at MOST 10 letters.\n";
cout << "\tNames must NOT contain spaces.\n\n\n\n\n\n";

system ("pause");
system ("cls");

cout << "\t\t\t\t\t\tTinyBit Games Presents to you:\n";
cout << "\n\t\t\t\t\t\t           Torn\n";
/*
In the future code a "Load" Option and a "New Game" Option
Also code a "Save" option that can be done at any time
Remember this is a TEXT ADVENTURE not any other game
*/

system ("pause");//Game Begins Here Still Refrain from goto statements
system ("cls");

One:
cout << "\tIt's Dark... Pitch Black. A throbbing headache bursts into the front of your head.\n";
cout << "As you begin to get up from where you are you realize that you are bound by strong\n";
cout << "rope.\n\n";
cin >> Answer;
if (Answer == "wait"){
    cout << "You wait... the time is unknown\n\n";
}else {
    cout << "\n\n\nThat answer is not a usable answer in this situation\n\n\n";
    goto One; //I didn't know what else to use please critique!
}
cout << "\tYou hear footsteps faintly, distant but audible. They are getting louder and they\n";
cout << "suddenly stop. A door creaks open and you hear it touch a wall\n\n\n";

system ("pause");
return 0;
}

I see you've ignored the advice from your previous question. To avoid duplication, I'm going to quote them:

From this answer:

  Goto

  At a first approximation, never use goto. It makes it more or less
  impossible to follow code. Without goto, you also don't need labels.
  Instead, just call different functions. Or keep an internal state
  machine. Or really any possible other solution. Code with goto like
  this is incredibly brittle - what if you want to add more states
  somewhere in between? Good luck keeping track of everything.

From this answer:

  There many things here that can be improved. First off, let's tackle
  some best practices. Generally, doing stuff like this with system
  isn't very good:

system("pause");
system("cls");

  To start off, system("pause"); is a bad way of "pausing" for user
  input. First off, there's a decently sized overhead just to execute a
  shell command, and doing this is rather insecure. If you really need
  to pause on user input, and ignore it, then just do something like
  this:

std::cin.get();

  Secondly, putting anything like this at the top of your file is a bad
  idea:

using namespace std;

  This pollutes your local namespace with everything in std, and it
  can lead to some name conflicts, especially if you're using a library
  like boost. There's a good reason it's named std and not
  standard. It's easier to prefix everything with std:: rather than
  standard::.

Now for some advice specific to this question:

First, all the newlines and tabs are quite noisy. You can consider putting it in a variable:

std::string tab(6, '\t');
std::cout << tab << "...";

Secondly, instead of your goto approach, it's more likely that you want a while loop instead. Some users insist that the while loop is a glorified goto, but without the negatives.

std::string answer;
while (std::cin >> answer)
{
    // ...
}


Linked List with c++ class I am playing with a sort of linked list using C++ classes. I have used a lot of C linked lists (struct based) during C programming course, but there was no behavior. Thus, I am a bit confused about the correct definition of a class containing a pointer to another object of the same class.
Here is the header:

#ifndef CONNECTEDPOINT_H
#define CONNECTEDPOINT_H

#include "vector3d.h"

class ConnectedPoint
{
  using Position = Vector3D<double>;

public:
  ConnectedPoint(unsigned index, const Position& pos);
  ~ConnectedPoint();
  ConnectedPoint(const ConnectedPoint& point);
  ConnectedPoint& operator=(const ConnectedPoint& point);

  void connectTo(const ConnectedPoint& point);

  unsigned index() const {return m_index;}
  const Position& position() const {return m_position;}
  Position& position() {return m_position;}
  ConnectedPoint* getNext() {return next;}

private:
  unsigned m_index;
  Position m_position;
  ConnectedPoint* next;
};
#endif // CONNECTEDPOINT_H

and the implementation file:

#include "connectedpoint.h"
ConnectedPoint::ConnectedPoint(unsigned index, const ConnectedPoint::Position& pos)
  : m_index(index)
  , m_position(pos)
  , next(nullptr)
{

}

ConnectedPoint::~ConnectedPoint()
{
  if(next != nullptr)
  {
    delete next;
    next = nullptr;
  }
}

ConnectedPoint::ConnectedPoint(const ConnectedPoint& point)
  : m_index(point.m_index)
  , m_position(point.m_position)
{
  if(point.next == nullptr)
  {
    this->next = nullptr;
  }
  else
  {
    next = new ConnectedPoint(*(point.next));
  }
}

ConnectedPoint& ConnectedPoint::operator=(const ConnectedPoint& point)
{
  if(this == &point)
  {
    return *this;
  }

  delete next;
  next = nullptr;

  m_index = point.m_index;
  m_position = point.m_position;

  if(point.next != nullptr)
  {
    next = new ConnectedPoint(*(point.next));
  }

  return *this;
}

void ConnectedPoint::connectTo(const ConnectedPoint& point)
{
  next = new ConnectedPoint(point.index(),point.position());
  *(next->next) = *(point.next);
}

Vector3D is just a class template which holds 3 coordinates and allows standard mathematical vector operations. Here its behaviors are not used.

Basically I would like to know:

Is this a proper implementation for such a class? Are there mistakes both in the approach and in the implementation?

Only see 1 bug:
void ConnectedPoint::connectTo(const ConnectedPoint& point)
{
  next = new ConnectedPoint(point.index(),point.position());
  *(next->next) = *(point.next);
}

In other places where you connect things (copy constructor/assignment) you delete the list of object pointed at by next before overwriting. You don't do that here. So if next is not a nullptr then you will leak.
Simplify
Your code is based on the fact that the next pointer owns a unique version of the object it points at. Whenever you do any manipulation of lists you preserve this by making sure you dynamically create copies of the object (which is fine). But there is already a class that will do this for you.
 std::unique_ptr<ConnectedPoint>  next;

std::unique_ptr is not only designed to correctly do ownership semantics by using you are explicitly telling everybody that you are using ownership semantics and they need to read less of your code to understand the semantics of your object.
Assignment does not provide strong gurantee
ConnectedPoint& ConnectedPoint::operator=(const ConnectedPoint& point)
{
  if(this == &point)
  {
    return *this;
  }

  // Delete the old state
  delete next;
  next = nullptr;

  // STUFF

  // What happens if this fails?
  next = new ConnectedPoint(*(point.next));

  // If an exception is generated (it may be caught higher up in the stack).
  // At this point your object now has lost the state it used to have and
  // never got the new state. So the behavior of your code could be eratic
  // after this point.

  // MORE STUFF
}

The Strong Exception gurantee basically says either the operation works correctly OR (the operation fails with an exception AND the object is unchanged). TO do this you need to use a more conservative technique. But its easy. You just use what is called the copy and swap idium.
ConnectedPoint& ConnectedPoint::operator=(const ConnectedPoint& point)
{
    ConnectedPoint copy(point);
    copy.swap(*this);
    return *this;
}
ConnectedPoint::swap(ConnectedPoint& rhs)
{
    using std::swap;
    swap(m_index,    rhs.m_index);
    swap(m_position, rhs.m_position);
    swap(next,       rhs.next);
}

The first thing people notice is that there is no test for self assignment.
  if(this == &point)
  {
    return *this;
  }

This is because it is no longer needed as we always make a copy. Sure this makes it less efficient when we do self assignment (add it back in if you really want). But self assignment though possible is actually very rare. So you are optimizing for a very rare condition at the expense of the normal operation. So normal operations are slowed down (the thing that happens all the time) while we optimize for things that are unlikely to happen.


Find a bitmap within another bitmap I have a function that will try to find a bitmap within another bitmap.

If it successfully finds it, then it will return the center point of bitmap #1.

I've only seen negativity towards goto which is what this function relies on.

I would like some help optimizing my function and possibly having it work without the need of goto.

private static bool FindBitmap(Bitmap searchBitmap, Bitmap withinBitmap, out Point point)
{
    for (var outerX = 0; outerX < withinBitmap.Width - searchBitmap.Width; outerX++)
    {
        for (var outerY = 0; outerY < withinBitmap.Height - searchBitmap.Height; outerY++)
        {
            for (var innerX = 0; innerX < searchBitmap.Width; innerX++)
            {
                for (var innerY = 0; innerY < searchBitmap.Height; innerY++)
                {
                    var searchColor = searchBitmap.GetPixel(innerX, innerY);
                    var withinColor = withinBitmap.GetPixel(outerX + innerX, outerY + innerY);

                    if (searchColor.R != withinColor.R || searchColor.G != withinColor.G ||
                        searchColor.B != withinColor.B)
                    {
                        goto NotFound;
                    }
                }
            }
            point = new Point(outerX, outerY);
            point.X += searchBitmap.Width / 2; // Set X to the middle of the bitmap.
            point.Y += searchBitmap.Height / 2; // Set Y to the center of the bitmap.
            return true;

            NotFound:
            continue;
        }
    }
    point = Point.Empty;
    return false;
}

Accessing the Pixels

GetPixel() is notoriously slow for iterating through the whole bitmap. Consider using LockBits for accessing the pixels directly in memory. After making sure the alpha channel is the same on both bitmaps (should be automatically unless one of the images is transparent), simply compare integer values for each pixel instead of manually comparing R then G then B.

Improving the Search

I would also adopt a faster algorithm for doing the search: Take the first line of the second bitmap (needle) and check on which lines of the first bitmap (haystack) that line is matching. Afterwards, simply check if all the other lines of the needle are placed immediately below the first one => much less comparisons to perform.

Return value

Returning a bool only to request another value be passed as an out parameter is troublesome. Simply return the Point you found wrapped inside Nullable<> or null if the search returned nothing. Also, I would rather have the function return the top of bottom left corner of where the second image was found so that I can perform some operation using that coordinate instead of getting the center point with which I do little without extracting the corner anyway.

Sample Code

Here is an example code of the idea presented:

public Point? Find(Bitmap haystack, Bitmap needle)
{
    if (null == haystack || null == needle)
    {
        return null;
    }
    if (haystack.Width < needle.Width || haystack.Height < needle.Height)
    {
        return null;
    }

    var haystackArray = GetPixelArray(haystack);
    var needleArray = GetPixelArray(needle);

    foreach (var firstLineMatchPoint in FindMatch(haystackArray.Take(haystack.Height - needle.Height), needleArray[0]))
    {
        if (IsNeedlePresentAtLocation(haystackArray, needleArray, firstLineMatchPoint, 1))
        {
            return firstLineMatchPoint;
        }
    }

    return null;
}

private int[][] GetPixelArray(Bitmap bitmap)
{
    var result = new int[bitmap.Height][];
    var bitmapData = bitmap.LockBits(new Rectangle(0, 0, bitmap.Width, bitmap.Height), ImageLockMode.ReadOnly,
        PixelFormat.Format32bppArgb);

    for (int y = 0; y < bitmap.Height; ++y)
    {
        result[y] = new int[bitmap.Width];
        Marshal.Copy(bitmapData.Scan0 + y*bitmapData.Stride, result[y], 0, result[y].Length);
    }

    bitmap.UnlockBits(bitmapData);

    return result;
}

private IEnumerable<Point> FindMatch(IEnumerable<int[]> haystackLines, int[] needleLine)
{
    var y = 0;
    foreach (var haystackLine in haystackLines)
    {
        for (int x = 0, n = haystackLine.Length - needleLine.Length; x < n; ++x)
        {
            if (ContainSameElements(haystackLine, x, needleLine, 0, needleLine.Length))
            {
                yield return new Point(x, y);
            }
        }
        y += 1;
    }
}

private bool ContainSameElements(int[] first, int firstStart, int[] second, int secondStart, int length)
{
    for (int i = 0; i < length; ++i)
    {
        if (first[i + firstStart] != second[i + secondStart])
        {
            return false;
        }
    }
    return true;
}

private bool IsNeedlePresentAtLocation(int[][] haystack, int[][] needle, Point point, int alreadyVerified)
{
    //we already know that "alreadyVerified" lines already match, so skip them
    for (int y = alreadyVerified; y < needle.Length; ++y)
    {
        if ( ! ContainSameElements(haystack[y + point.Y], point.X, needle[y], 0, needle.Length))
        {
            return false;
        }
    }
    return true;
}

I've also done a benchmark for finding a 246x228 pixel needle located at (169,  281) in an 800x600 pixel haystack:

Optimized method: ~ 11 ms

Initial method:  ~ 261 ms


JS function to initialize properties I have some JavaScript code to manage a 2D map in a game. I have a function to create a new room in the map which is at map[x][y]. I need to create that property and also initialize the monsters property. The code I wrote looks like this, which works fine.
'use strict';

let map = {};

function newRoom(x, y) {
    if (map[x] === undefined)
        map[x] = {};
    map[x][y] = {};
    map[x][y].monsters = {};
}

newRoom(0, 0);

Is there a more concise way to write code like this?

Fundamentally, not really. When you need to create a nested structure, there are no built-in ways to get around explicitly testing if the intermediate object(s) exist, and creating them if they don't.
(If you just wanted to access a possibly-existing nested element, you could use optional chaining to achieve it concisely, but nested creation is much more verbose)
There are libraries that do nested creation for you, but they ultimately implement the same sort of logic under the hood:

// Just for demonstration, I do not recommend using a library for this:

let map = {};
function newRoom(x, y) {
  _.setWith(map, [x, y, 'monsters'], {}, Object);
}
newRoom(0, 0);
console.log(map);
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.20/lodash.min.js"></script>

One shortcut you can make in your original code is to simply check map[x] instead of comparing against undefined. You can also declare the monsters object inline when assigning the room. You should also declare the map object with const unless you're reassigning it later.
Now that logical nullish assignment exists, you could use it as well, though given how new it is, you (or others who may read the code) may not be comfortable with it, and it may look a bit confusing regardless:

const map = {};

function newRoom(x, y) {
    map[x] ??= {};
    map[x][y] = {
        monsters: {}
    };
}

/*
or
function newRoom(x, y) {
    if (!map[x]) {
      map[x] = {};
    }
    map[x][y] = {
        monsters: {}
    };
}
*/

newRoom(0, 0);
console.log(map);

I do recommend using brackets for your if blocks. They'll make it a bit easier to read and will keep you from accidentally writing on the wrong indentation level assuming that something is inside the if block, when it actually isn't.
Note that both the above code and your original code will overwrite contents of the room if it already exists in the map. Hopefully that's desirable.
There are a few alternative methods to achieve the same result that require fewer characters of code, that code golfers or minifiers could use, but they sacrifice readability and therefore aren't worth it IMO, eg:

const map = {};

function newRoom(m,n){map[m]||(map[m]={}),map[m][n]={monsters:{}}}

newRoom(0, 0);
console.log(map);

Another thing to consider - as of ES2015, Maps are more suited to dynamic properties than objects. If you want a Map, use a Map instead of an object.
function newRoom(x, y) {
    if (!map.has(x)) {
        map.set(x, new Map());
    }
    map.get(x).set('y', { monsters: {} });
}


IP address exclusion algorithm The below algorithm is supposed to take a list of input IPs/networks and a list of IPs/networks to exclude, and return a list of networks that don't include any of the IPs/networks to exclude...
Please review it and provide me with some feedback!
import ipaddress
from ipaddress import IPv4Network

def recurse_exclude(supernet_list, exclude_list):
    # For some reason, this only works if we force the generators into lists
    for supernet in list(supernet_list):
        for exclude in exclude_list:
            try:
                excluded_list = recurse_exclude(list(supernet.address_exclude(exclude)), exclude_list)
            except ValueError:
                # Ignore when the IP/net to exclude was not in the supernet
                continue
            else:
                return list(excluded_list)
    return supernet_list

supernet_list = [IPv4Network('1.1.0.0/24')]
output = recurse_exclude(supernet_list, exclude_list=[IPv4Network('1.1.0.0/25'),IPv4Network('1.1.0.128/26')])

Review

Python and recursion are not the best match, instead use loops
It could even be done with list comprehension
return [ip if ip not in exclude for ip in subnet]

Alter your imports to use one of the two not both
I suggest to use from ipaddress import IPv4Network because that seems to be only thing imported from the library.

Use sets!
Lists are O(N), while sets are O(1)!
Secondly it's quite easy to subtract sets simply use set(c) = set(a) - set(b)

Example
from ipaddress import IPv4Network

exclude = set(IPv4Network('1.1.0.0/25')) | set(IPv4Network('1.1.0.128/26'))
net = set(IPv4Network('1.1.0.0/24'))

print(net - exclude)


Length of longest palindrome subsequence I had an interview, where I was asked to solve to this question:

Given an string str, find the maximum length of palindrome subsequence.
Example: >if str = 'abcda':
maximum_length = 3 'aca'

How can I improve this code?
def maximum_length_palindrome_subsequence(a):
    a = list(a)
    n = len(a)
    table = [[0 for i in xrange(n)] for j in xrange(n)]
    for i in xrange(n):
        table[i][i] = 1
    for gap in xrange(1,n):
        for i in xrange(n-1):
            h = i+gap
            if h>=n:
                continue
            if a[i] == a[h]:
                if i+1==h:
                    table[i][h] = 2
                else:
                    table[i][h] = table[i+1][h-1] + 2
            else:
                if h<n:
                    table[i][h] = max(table[i+1][h], table[i][h-1])
    return table[0][n-1]

print maximum_length_palindrome_subsequence('abcbda')

Back to the basic of code review, what does this code do and how do you do it. At first glance I've got not clue what so ever this code does. I don't know what the content of the table is. I don't know what the h or the gap is referring to, and I don't know why the table[0][n-1] is the correct answer.

The only thing I know is that your function is supposed to return the "maximum length palindrome subsequence" of a. Which is not entirely clear either. (I wrote an answer (now deleted) based on a misconception of the terms included, as I didn't fully realize the definition of subseqence)

So let us review some unknown code:

Vague variable naming – The variable could strongly benefit from indicating what they are actually containing. Like instead of a, it could be sourceString or source or text. The i and n is commonly known as the loop counter, and the number of elements. The a, gap, h and table, no idea what they contain. Especially the table is badly named.
No comments – It would have been nice to see some comments describing what the various loops are trying to achieve, or how the table is initialized. For the method possibly some examples of valid responses.
Break out early – As in the answer by @Dair, since i is ever increasing, whenever the h >= n you can do a break.
Strange comparison, no I – You compare a[i] == a[h], why? If i == h this will always be true. However h = i + gap, so this does essentially say a[i] == a[i + gap] which would be a little bit clearer, although I still don't quite get why we compare this.
Strange comparison, no II – Right below you compare i+1 ==h, which is the same as i + 1 == i + gap, which can be simplified to gap == 1.
Unneeded if h<n – If you get this far, then it's a given that h < n, as otherwise you'd continue'd. Hence the if could be removed all together.

Based on the code so far I'm guessing that table holds the length of the palindrome starting at that first index and ending at last index. Not quite how it gets there, but that is my assumption. Let us rewrite the code according to this, and my comments above:

def maximum_length_palindrome_subsequence(text):
    """Search through text to find the maximum length of a
       palindrome looking at subsequences of the text.
         text = "abcda" -> palindrome: aca -> length = 3
         text = "xaybzcydxe" -> palindrome: xyzyx -> length = 5
    """
    text_length = len(text)

    # palindrome_length is a two dimensional table of palindrome
    # lengths where the first dimension is start index, and the
    # second dimension is the end index of the palindrome in text

    # Initialize with all zeroes
    palindrome_length = [[0 for i in xrange(text_length)] for j in xrange(text_length)]

    # As a base, every single character through text is a
    # palindrome of length 1
    for i in xrange(text_length):
        palindrome_length[i][i] = 1

    # Loop through the various gaps in text, and check for
    # similarity for every index of the text. Update the
    # palindrome_length accordingly if matching characters
    for gap in xrange(1, text_length):
        for i in xrange(text_length - 1):

            # h is the candidate index? (or something like that?!)
            h = i + gap
            # If candidate index is after length of text, break out
            if h >= text_length:
                break

            # If equal characters, update palindrome_length
            if a[i] == a[h]:
                if gap == 1:
                    palindrome_length[i][h] = 2
                else:
                    palindrome_length[i][h] = palindrome_length[i + 1][h - 1] + 2
            else:
                # Characters not equal, so choose a max based upon
                # ... some criteria ...
                palindrome_length[i][h] = max(palindrome_length[i + 1][h],
                                              palindrome_length[i][h - 1])

    # Return the maximum length based upon entire text
    return palindrome_length[0][n - 1]

print maximum_length_palindrome_subsequence('abcda')

Based upon this rewrite I've got some additional comments:

Simplify initialization of palindromeLength – Instead of first filling it with 0, and then refilling parts with 1, you could do:

palindrome_length = [[ 1 if i == j else 0
                         for i in xrange(text_length)]
                            for j in xrange(text_length)]

Criteria for the max() – Is this somewhat related to palindromes of an even or odd length? That it chooses the longest of those two alternatives?

To conclude, your code does follow proper indentation, is a little lacking in space around operators, but the main issue is bad naming and unclear intentions of the various code blocks. There is some magic happening in this code, which could benefit from clear comments sprinkled out in the code.

  Imaging what you yourself would think of your code if it was presented to you as it stood originally, but with the name function_x and the question: What does function_x do? Would you be able to answer it?

If my assumption on the max is correct, my final version is:

def maximum_length_palindrome_subsequence(text):
    """Search through text to find the maximum length of a
       palindrome looking at subsequences of the text.
         text = "abcda" -> palindrome: aca -> length = 3
         text = "xaybzcydxe" -> palindrome: xyzyx -> length = 5
    """
    text_length = len(text)

    # palindromeLength is a two dimensional table of palindrome
    # lengths where the first dimension is start index, and the
    # second dimension is the end index of the palindrome in text

    # Every character is considered a palindrom of length 1, whilst
    # the rest is initialized to 0
    palindrome_length = [[ 0 if i != j else 1
                             for i in xrange(text_length)]
                                 for j in xrange(text_length)]

    # Loop through the various gaps in text, and check for
    # similarity for every index of the text. Update the
    # palindrome_length accordingly if matching characters
    for gap in xrange(1, text_length):
        for i in xrange(text_length - 1):

            # h is the candidate index? (or something like that?!)
            h = i + gap
            # If candidate index is after length of text, break out
            if h >= text_length:
                break

            # If equal characters, update palindrome_length
            if text[i] == text[h]:
                if gap == 1:
                    palindrome_length[i][h] = 2
                else:
                    palindrome_length[i][h] = palindrome_length[i + 1][h - 1] + 2
            else:
                # Characters not equal, so choose a max based upon
                # the even or odd length palindrome length?
                palindrome_length[i][h] = max(palindrome_length[i + 1][h],
                                              palindrome_length[i][h - 1])

    # Return the maximum length based upon entire text
    return palindrome_length[0][text_length - 1]

print maximum_length_palindrome_subsequence('abcda')           # "3"
print maximum_length_palindrome_subsequence('xaybzcydxe')      # "5"
print maximum_length_palindrome_subsequence('abcdaabcda')      # "6"
print maximum_length_palindrome_subsequence('racecar')         # "7"
print maximum_length_palindrome_subsequence('racecars')        # "7"
print maximum_length_palindrome_subsequence('iiiiragceciars')  # "7"

At the very end I presented a few more cases, to verify the correctness of the revised version.

Update: Changed to snake_case to conform to standards. Hopefully I didn't break the code. :-)


Speed up anagram method in Ruby I'm working on a project to find anagrams of words. My goal is to figure out the longest anagrams out there. I took a look at my code and it works but is woefully inefficient.

I have two arrays, each containing a length of long words:

words_one = ["sarcocarcinomata", "dactylopteridae", "autoradiographic", "pterylographical",
"nonconversational", "pathophysiological"]

words_two = ["carcinosarcomata", "pterodactylidae", "radioautographic", "pterylographical", "nonconservational", "physiopathological"]

Most of these words have to do with science, so they're relatively long.

For what I'm working on, I would like to find if all of the word pairs are anagrams of each other. In my example above, they are.

def anagrams(words_one, words_two)
  x = 0
  while x < words_one.count
    if words_one[x].chars.to_a.permutation.include?(words_two[x].chars.to_a)
      puts 1
      x += 1
    else
      puts 0
      x += 1
    end
  end
end

There are several potential problems I see here:

Permutation Method: This sorts through all permutations of a given string. Each letter can be one of 26 different characters. This adds up real quickly.
While Loop: I'm not sure this is the most effective method.
Duplicate Code: I put x+=1 in both sections of my if-else block.

I'd like to learn some new techniques to optimize Ruby code.

Which type of algorithm is my code and which type of algorithm do I need?

While the formulation Does any permutation of word A's letters match word B? is technically correct, it's also terribly impractical when implemented. As you yourself point out, there are a lot of permutations, and we're looking for just one.

A simpler formulation would be

  Are the letters in word A the same as in word B?

Or, in code

word_a.chars.sort == word_b.chars.sort

That'll be a lot quicker.

Also, since you've got 2 arrays, and want to match one word from each at a time, you could use Array#zip to produce a single array of pairs, and loop that instead. You rarely need to deal with plain C-style loops or actual array indexes in Ruby, because you have all the methods from Enumerable. It's not that these are necessarily more efficient than old-school loops, it's that they are more expressive and much more idiomatic.

One of those Enumerable methods, by the way, is all? which simply checks if a block evaluates to non-false/non-nil for all items.

So for instance, you could do this:

def all_anagrams?(words_one, words_two)
  words_one.zip(words_two).all? do |word_a, word_b|
    word_a.chars.sort == word_b.chars.sort
  end
end

Basically: pair up the words from each array, and check if the words in each and every pair are anagrams of each other.

And yes, it returns true for the two arrays in your question.

Of course, it doesn't say much about what words (if any) aren't anagrams. If you want more feedback, you could look into using #detect to find the first pair that isn't anagrammatic (if no such pair is found, well, they are all anagrams). Or you could use #partition to divide the list of pairs into two lists: Those that are anagrammatic, and those that are not.

In any case, I'd recommend against printing anything in a method, unless it's stated purpose is indeed to print stuff. Much better to have more methods and combine their functionality, than have a few methods do many things.

Speaking of methods, the current name of your method, anagrams, doesn't quite say what it does. Does it return some random anagrams? Does it produce them based on input? Does it verify them? Typically, when a method just returns a boolean value, its name is suffixed with a ? in Ruby. Makes it nice and obvious that you can expect a yes/no answer.
I've tried for a slightly more descriptive name above with the all_anagrams? name (it isn't perfect either, though).

Oh, and yes, the duplication of x += 1 is problematic. You can just move it to appear after the if..else. It's doubly wise to do so here, since you'll have an infinite loop if x doesn't get incremented as expected. So it's better to at least have the increment happen in only 1 place, than to hope it happens in several, branching code paths.


99 bottles of beer on the wall + EcmaScript 6 Today, a solution written in Python was posted to this challenge, and since I was inspired by it, I made a version in EcmaScript 6.

I tried to explore some new language constructs:

arrow functions
template Strings
let
for..of
generators

The code:

function* bottlesOfBeer(howMany=99){
  let pluralizeBottles = count => count !== 1 ? "bottles" : "bottle";
  let takeNext = next => next >= 1 ? next.toString() : "No more";
  for (let i = howMany; i >= 0; i--) {
    let bottle = pluralizeBottles(i);
    let bottleNext = pluralizeBottles(i-1);
    let left = takeNext(i);
    let toTake = i > 1 ? "one" : (i == 1 ? "it" : "no more");
    let toTakeNext = takeNext(i-1);
    let line1 = `${left} ${bottle} of beer on the wall, ${left.toLowerCase()} ${bottle} of beer.`;
    let line2 = `\nTake ${toTake} down and pass it around, ${toTakeNext.toLowerCase()} ${bottleNext} of beer on the wall.\n`;
    yield `${line1}${i > 0 ? line2 : ""}`;
  }
  yield `Go to the store and buy some more, ${howMany} bottles of beer on the wall.`;
}

for (let it of bottlesOfBeer()) {
  console.log(it);
}

This code can be copy/pasted into here to see it working: http://6to5.org/repl/

Did I use these constructs appropriately? I'd like to know if I somehow abused them.

And could it be made more readable?

What do you think, overall?

Style

There's a few inconsistent stylistic choices, mostly regarding whitespace.

 howMany=99                        // no spaces around "="
 let bottle = pluralizeBottles(i); // spaces around "="

Let

There's nothing wrong with the way you use let, but when talking about let you usually also talk about const since they're available since the same ES version.

You never mutate any of the variables you declare and initialize with let, which means they can all be changed to const.

// old
let bottle = pluralizeBottles(i);

// new
const bottle = pluralizeBottle(i);

This is also true for the for loop.
By using const, you ensure that you never do an assignment to it.

// old
for (let it of bottlesOfBeer()) {

// new
for (const it of bottlesOfBeer()) {

Helper functions

Do not put your helper functions inside the generator.
They will be created each time the generator is called -- there's no need for this.
Move them outside.
If you want to make them private, use modules (since we're talking about ES6), and just don't export them.

I'd also create more util functions.
For example, the following line is a bit too long and complex.
It's more complex than your pluralizeBottles function, so there's no reason why this shouldn't be extracted as well.

let toTake = i > 1 ? "one" : (i == 1 ? "it" : "no more");

Usage of generators

Your usage of generators is good.
This is the type of problem they're created for.


Accumulate ids to make a single ajax request I have multiple places where I need to make ajax requests to fetch the items corresponding to some ids. However, I only want to make a single request by accumulating these ids and debouncing the actual method that makes the ajax request...So far I've come up with this code, but it just feels ugly/non-reusable.

Is there any simpler/recommended method to achieve similar results without sharing resolve/promise variables like I did here?

Here's a fiddle

const fakeData = [{
    id: 1,
    name: 'foo'
  },
  {
    id: 2,
    name: 'bar'
  },
  {
    id: 3,
    name: 'baz'
  }
];

let idsToFetch = [];

let getItemsPromise, resolve, reject;

const fetchItems = _.debounce(() => {
  console.log('fetching items...');
  const currentResolve = resolve;
  const currentReject = reject;

  // simulating ajax request
  setTimeout(function() {
    const result = idsToFetch.map((id) => fakeData.find(item => item.id == id));
    currentResolve(result);
  }, 400);

  getItemsPromise = resolve = reject = null;
}, 500);

function getItems(ids) {
  idsToFetch = ids.filter((id) => !idsToFetch.includes(id)).concat(idsToFetch);
  if (!getItemsPromise) {
    getItemsPromise = new Promise((_resolve, _reject) => {
      resolve = _resolve;
      reject = _reject;
    });
  }

  fetchItems();

  return getItemsPromise
    .then((res) => {
      return res.filter((item) => ids.includes(item.id));
    })
}

setTimeout(() => {
  console.log('first request start');
  getItems([1]).then(res => console.log('first result:', res));
}, 100);
setTimeout(() => {
  console.log('second request start');
  getItems([1, 2]).then(res => console.log('second result:', res));
}, 200)
setTimeout(() => {
  console.log('third request start');
  getItems([1, 3]).then(res => console.log('third result:', res));
}, 300)
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.min.js"></script>

I was able to somehow encapsulate the logic be creating a function generator that holds the previous two functions like this:

  const fakeData = [{
      id: 1,
      name: 'foo'
    },
    {
      id: 2,
      name: 'bar'
    },
    {
      id: 3,
      name: 'baz'
    }
  ];

  function makeGetter(fetchFunc, debounceTime = 400) {
    let idsToFetch = [];

    let getItemsPromise, resolve, reject;

    const fetchItems = _.debounce(() => {
      console.log('fetching items...');
      const currentResolve = resolve;
      const currentReject = reject;
      const currentIdsToFetch = idsToFetch;

      Promise.resolve(fetchFunc(currentIdsToFetch))
        .then(res => currentResolve(res))
        .catch(err => currentReject(err));

      getItemsPromise = resolve = reject = null;
      idsToFetch = [];
    }, debounceTime);

    const getItems = (ids) => {
      idsToFetch = ids.filter((id) => !idsToFetch.includes(id)).concat(idsToFetch);
      if (!getItemsPromise) {
        getItemsPromise = new Promise((_resolve, _reject) => {
          resolve = _resolve;
          reject = _reject;
        });
      }

      fetchItems();

      return getItemsPromise
        .then((res) => {
          return res.filter((item) => ids.includes(item.id));
        })
    }

    return getItems;
  }

  const getItems = makeGetter((ids) => {
    // simulating ajax request
    return new Promise((resolve, reject) => {
      setTimeout(function() {
        const result = ids.map((id) => fakeData.find(item => item.id == id));
        resolve(result);
      }, 400);
    })
  });

  setTimeout(() => {
    console.log('first request start');
    getItems([1]).then(res => console.log('first result:', res));
  }, 100);
  setTimeout(() => {
    console.log('second request start');
    getItems([1, 2]).then(res => console.log('second result:', res));
  }, 200)
  setTimeout(() => {
    console.log('third request start');
    getItems([1, 3]).then(res => console.log('third result:', res));
  }, 300)
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.min.js"></script>


Checking for files that are mentioned as children of other files I am working in JavaScript on a GruntFile script, and I am creating an object that holds a file class for each one of the following class structure:

class file = {
    path: string,
    name: string,
    children: string[],
    parents: string[],
    packages: string[]
}

I need to loop over each one of these files in the object, and for each one of the files loop over all of the other files and check if their children array contains the current file matching on name.

My coworker and I have come up with multiple solutions to this nasty business.

The first is a more "streamlined" approach, which is believed to "execute faster":

const fileArray = Object.keys(files).map( key => files[key]);
fileArray.forEach( fileOuter => fileArray.forEach( fileInner => { if ( fileInner.children.includes(fileOuter.name)) fileOuter.parents.push(fileInner.name) } ) );

The second solution is clearer:

// Loop over all files by key
Object.keys( files ).forEach( key => {

    // Get the current file name
    const curFileName = file.name;

    // Loop over all files again
    Object.keys(files).forEach( innerKey => {

        // Get the inner loop file
        const fileInner = files[innerKey];

        // If the file lists the current file as a dependency
        if ( fileInner.children.indexOf(curFileName) !== -1 ){

            // Push the file name into the parents array for the current file
            files[curFileName].parents.push(fileInner.name);
        }
    })
})

Or if we really wanted to make the loop as fast as possible:

// Get the object keys of the files object
const fileKeys = Object.keys(files);

// Loop over each file
for( var i=0, j=fileKeys.length; i<j; i++){

    // Get the current file name
    const curFileName = files[fileKeys[i]].name;

    // Loop over each file
    for( k=0, k<j, k++){

        // Get the inner file
        const fileInner = files[fileKeys[k]];

        // If the inner files children array matches the current file name, push the inner file name to the current files parents array
        if( fileInner.children.indexOf(curFileName) !== 1 ) files[curFileName].parents.push( fileInner.name );
    }
}

Is any of these methods objectively better than the other? Is there an even better approach we could be taking for this task?

General Remarks

Why are your files assigned to properties of an object? Keeping them in an array would be simpler. Or do you wish to store some kind of mapping from id → file? You currently use the file's name as a unique id which would then redundantly appear on both sides of that mapping.

Matching only by file name seems risky to me, but it really depends on your definition of 'child'. Depending on the exact use case, a file path plus name or a file hash value might be a more suitable unique identifier.

I would avoid talking about 'class structure' when describing the structure of anonymous objects. 'class' has a specific meaning, especially since the class keyword has been introduced to JavaScript now. If your files were really instances of a class, you would probably want to use some kind of creational design pattern such as a factory or factory method which avoids the issue of having to deal with incompletely initialized instances until you finally fill in the parents.

Style

The first code sample could be more readable if it weren't squeezed into a single line.

All code samples could benefit from better naming. Instead of file write child, instead of fileInner write parent, instead of curFileName write child.name etc.

Instead of indexOf use includes. Since you already use const, you should also use let instead of var or globals.

Instead of Object.keys use Object.values.

Your first or second example could be written as:
files.forEach(parent => {
  files.forEach(child => {
    if (parent.children.includes(child.name)) {
      child.parents.push(parent.name);
    }
  });
})

Alternatively using for-of-loops:
for (const child of files) {
  for (const parent of files) {
    if (parent.children.includes(child.name)) {
      child.parents.push(parent.name);
    }
  }
}

If files is not an array but an object, replace files with Object.values(files).
Performance
If performance is really an issue, look into the runtime complexity of your operations. By introducing an explicit Map from parent names to children files, the runtime complexity could be reduced to depend linearly on the total number of children only.


Random changes in a list of binary numbers I have a list of N integers, e.g. intList=[1,7,3,1,5] with N=5 and all integers have a binary representation of lenght l, e.g. binList=["001","111","011","001","101"] with l=3. Now I want to add random flips from 1->0 & 0->1. Each of the l positions should flip with a probabiliy mu. (mu varies over many magnitudes & N is around 100-100000) This happens over many iterations. Currently my code is
@nb.njit()
for x in range(iterations):
    ...other code...
    for number in range(N):
        for position in range(l):
            if random.random() < mu:
                intList[number] = intList[number] ^ (1 << position)
    ...other code...

The numba decorator is necessary. Since this part takes most of the execution time I just want to make sure that it is already optimal. Thanks for any comment!

You could replace
        for position in range(l):
            if random.random() < mu:
                intList[number] = intList[number] ^ (1 << position)

with
        for bit in bits:
            if random() < mu:
                intList[number] ^= bit

after preparing bits = [1 << position for position in range(l)] before the show and importing the random function.
Don't know whether that helps when using numba, and you didn't provide benchmark code.


Manipulating /etc/hosts file using Go This is my first go at a Go program that's not a variation of "Hello {$name}". It's about twice as long as my previous Python implementation, and perhaps an order of magnitude messier.

The goal is to read in /etc/hosts and combine the various entries I've made over time:

127.0.0.1 host1.test
127.0.0.1 host2.test
111.111.111.111 staging.dev otherproject.dev
127.0.0.1 localhost host.test

into:

127.0.0.1 host.test host1.test host2.test localhost
111.111.111.111 otherproject.dev staging.dev

Any thoughts on how to make this cleaner, better, and/or more idiomatic?

package main

import (
    "os"
    "io/ioutil"
    "strings"
    "sort"
)

// Because this doesn't exist?
type StringSet struct {
    set map[string]bool
}

func NewStringSet() StringSet {
    return StringSet{ make(map[string]bool) }
}

// Returns true if added, false if already exists
func (s StringSet) Add(str string) bool {
    _, found := s.set[str]
    s.set[str] = true
    return !found
}
// Returns the hostnames in alphabetical order
func (s StringSet) GetHostnames() []string {
    keys := make([]string, 0, len(s.set))
    for key := range s.set {
        keys = append(keys, key)
    }
    sort.Strings(keys)
    return keys
}

// Because this doesn't exist?
func pop(s []string) (string, []string) {
    var x string
    x, s = s[0], s[1:len(s)]
    return x, s
}

// Returns the lines of a file
func GetLines(filename string) []string {
    raw, err := ioutil.ReadFile("/etc/hosts")
    if err != nil {
        panic(err)
    }
    return strings.Split(string(raw[:]), "\n")
}

func WriteHostsFile(outfile string, hosts map[string]StringSet) bool {
    // Range outputs a random order -.-
    fd, err := os.Create(outfile)

    if err != nil {
        panic(err)
    }

    defer fd.Close()

    fd.WriteString("# Managed by hosts.go\n")

    for k := range hosts {
        names := hosts[k].GetHostnames()
        line := k + " " + strings.Join(names, " ")
        fd.WriteString(line + "\n")
    }

    return true
}

func ProcessHosts(data []string) map[string]StringSet {
    hosts := make(map[string]StringSet)
    for _, line := range data {
        var ip string
        parts := strings.Split(line, " ")

        if len(parts) > 1 {
            ip, parts = pop(parts)
            if ip[0] == '#' {
                continue
            }

            // Check if nil map
            if len(hosts[ip].set) == 0 {
                hosts[ip] = NewStringSet()
            }

            for _, word := range parts {
                hosts[ip].Add(word)
            }
        }
    }
    return hosts
}

func main() {
    data := GetLines("/etc/hosts")
    hosts := ProcessHosts(data)
    WriteHostsFile("hosts", hosts)
}

First pass on your code:

Don't panic. For example, your WriteHostsFile function should return error instead of bool.
Don't create a special struct for a string set. Just use map[string]bool and exploit zero values =) Don't be afraid to use map[string]map[string]bool to represent your hosts.

Your Add function go away, but fyi: you are making it return a bool and never using it.

Your pop function could be simplified and inlined. Just use ip, parts = parts[0], parts[1:].
Check for nil value using foo == nil instead of len(foo) == 0.
strings.Split(line, " ") only works if things are separated with spaces — but indentations are a perfectly valid separator for /etc/hosts =)
If your lines aren't too long (and you can probably assume lines from /etc/hosts are going to be smaller than 65k chars), use bufio.Scanner to read a file line by line. See this SO answer for a full example and a discussion of caveats.

Your GetLines function should probably be inlined, but fyi: you're silently discarding its argument.

You probably want to use flags for the input and output paths, and output on stdout by default (maybe have a flag to override /etc/hosts). Outputting to a file named hosts in the current directory is pretty unexpected.
Instead of for k := range hosts and then calling hosts[k], you can use directly for k, h := range hosts and use h.
You don't want to write into a file line by line — what if you're overwriting the original file and your binary fails in the middle? You probably want to have WriteHostsFile write into a bytes.Buffer and write everything at once at the end.
All your functions are exported (they start with an uppercase letter), but this is not a library; so you shouldn't export anything: they should all start with a lowercase letter.
Your algorithm is not deterministic, because you're iterating over the elements of a map in WriteHostsFile: it might be unexpected to see the file modified if all hosts are already correctly grouped. Sorting the hosts by key; or by first appearance in the original file, would solve this.

Cheers!


Menu items to enter and exit full-screen mode in JavaFX I need to improve this by using switch statement in which I want to set the text and setFullScreen. Is there any more elegant way to implement this?

final String change[] =
    {
        "Full Screen", "Exit Full Screen"
    };
    final MenuItem fullScreen = MenuItemBuilder.create().text(change[0]).build();

    fullScreen.setOnAction(new EventHandler<ActionEvent>()
    {
        @Override
        public void handle(ActionEvent e)
        {
            fullScreen.setText((fullScreen.getText().equals(change[0])) ? change[1] : change[0]);

            if (fullScreen.getText().equals(change[0]))
            {

                primaryStage.setFullScreen(false);
            }
            else
            {

                primaryStage.setFullScreen(true);
            }

        }
    });

    view.getItems().add(fullScreen);

This can be made a lot more elegant with the State Pattern.

We'll introduce an enum for the two states :

private static enum Mode {
    FULLSCREEN("Exit Full Screen"), NORMAL("Full Screen");

    private final String toggleActionText;

    private Mode(String text) {
        this.toggleActionText = text;
    }

    public String getToggleActionText() {
        return toggleActionText;
    }

    public boolean isFullScreen() {
        return FULLSCREEN == this;
    }

    public Mode other() {
        return isFullScreen() ? NORMAL : FULLSCREEN;
    }
}

Then, having added a properly initialized Mode field to the class, you can simplify your code to this :

final MenuItem fullScreen = MenuItemBuilder.create().text(mode.getToggleActionText()).build();

fullScreen.setOnAction(new EventHandler<ActionEvent>()
{
    @Override
    public void handle(ActionEvent e)
    {
        mode = mode.other();
        fullScreen.setText(mode.getToggleActionText());
        primaryStage.setFullScreen(mode.isFullScreen());
    }
});

view.getItems().add(fullScreen);

As you can see, it is a lot simpler to delegate the state dependent behavior to the Mode instance than to use an if whenever you have state dependent behavior.


Ruby script to deactivate or add users to database table I have a Ruby script meant for some basic database tasks, such as add a user and deactivate a user.  The end user can load up the script and follow the basic path of "Do I want to add a user?", or "Do I want to deactivate a user?". They have the option to retreat back to the main menu if needed.

The script works just fine, but being new to Ruby, I am starting to sense I have made things overly complicated and some simple refactoring could make the code much cleaner.  I am also interested to hear any other feedback.

  require 'yaml'

  config = YAML.load_file('config.yml')  ## Loads in the config file with databases eligible for selection

  puts 'Loading configured Databases'

  config.each do |k, v|
    puts " #{k}, #{v}"
  end

  def path  # Method to create a reusable menu
    puts "Press 1 to add a new user\nPress 2 to mark a user as active or inactive \nPress 3 to Exit"

    path = gets.chomp.to_i

    if path == 3
      puts 'Exiting...'
      exit
    else
      path_switch(path)
    end
  end

    def path_switch(path_var)
      config = YAML.load_file('config.yml')  ## Loads in the config file with databases eligible for selection
      path = path_var
      rds = ''
      table = ''
      choicelen = config.length
    case path
      when 1
        puts 'Select the numeric ID of the ICE installation to add the user to\n Enter 0 to go back'
        config.each do |k, v|
          puts "#{v['id']}: #{k}"
        end
          choice_client_ice = gets.chomp.to_i
           if choice_client_ice > choicelen
             puts 'error, not a valid selection'
             path
           end
          path if choice_client_ice == 0
          config.each do |k, v|
            if v['id'].to_i == choice_client_ice
              rds = v['rds']
              table = v['table']
            end
          end
            puts "Your current focus is: #{rds} at #{table}"
            path_add(rds, table)
      when 2
        puts 'Select the numeric ID of the ICE installation to edit the user for or type in 0 to go back'
        config.each do |k, v|
          puts "#{v['id']}: #{k}"
        end
          choice_client_ice = gets.chomp().to_i
          path if choice_client_ice == 0
          if choice_client_ice > choicelen
            puts 'error, not a valid selection'
            path
          end
          config.each do |k, v|
            if v['id'].to_i == choice_client_ice
              rds = v['rds']
              table = v['table']
            end
          end
            puts "Your current focus is: #{rds} at #{table}"
            path_edit(rds, table)
        else
          puts "Error, #{path} is not a valid option."
          path
      end
  end

  def path_add(rds, table)
    puts 'Input the users email address or enter the word exit to go back to the beggining'
    email = gets.chomp
    if email == 'exit'
      path
    else
    puts 'Please add the users group id'
    groupid = gets.chomp.to_i
    puts 'Please add the users course style id'
    coursestyleid = gets.chomp.to_i
    puts "This will create a user with email address #{email}, group id of #{groupid} and course style id of #{coursestyleid}.  Please confirm (y/n)"
    confirm = gets.chomp
    if confirm == 'y'
      ## Add in sequel call
      puts 'user has been added as xxx'
    else
      path
    end
  end
  end

  def path_edit(rds, table)
    puts 'Input the users ID or enter the word exit to go back to the beggining'
    uid = gets.chomp
    if uid == 'exit'
      path
    else
      puts 'The user xxx is currently xxx, this will mark him as xxx, continue? (y/n)'
      confirm = gets.chomp
        if confirm == 'y'
          ## Add in sequel call
          puts 'User has been marked as xxx'
        else
          path
        end
    end
  end

  def ice_select
  end

path

Some high level advice:

Fix your indendation.  For example, the indentation is randomly increased on this line: choice_client_ice = gets.chomp.to_i.  I mention this only because there is a lot of deep nesting here, which is confusing enough without incorrect indentation. Which brings me to...
Try to rewrite this entire thing, allowing yourself only 1 level of indentation (or 2, at most).  This will force you to separate high level logic and implementation detail, by pulling implementation out into its own functions.
Try to rewrite it, keeping your program logic and "views" separate.  All the puts statements are "view" code -- they are to your command line app what html files are to web apps.  Think about writing functions with names like main_menu_selection, which do nothing but present the user with his choices, get his selection, and return it.


URL Query params builder My goal is to create a URL query params from std::map<std::string, std::string>. For example, we call to_query_params( {{"one","1"},{"two","2"}} ) and we get ?one=1&two=2.

Is this a good way of doing it?

std::string to_query_params(std::map<std::string, std::string> map)
{
    if (map.empty()){
        return "";
    }
    auto map_beg = std::begin(map);
    auto concat = [](const decltype(map)::value_type& a){
                        return a.first+"="+a.second;
                    };
    std::string result("?"+concat(*map_beg));
    std::for_each(std::next(std::begin(map)),
                  std::end(map),
                  [&](decltype(map)::value_type param){
                      result+="&"+concat(param);
                  });
    return result;
}

You are not performing any percent encoding of parameter names or values, which would likely lead to breakage or even a security vulnerability if the you are given parameters that contain a special character.

A std::map might not be the right type; a std::multimap may be more appropriate.