Object construction differences Whenever I create a Javascript "class" I do something like this:

// Definition
function MyObject() {
   this.id = 0;
   this.legs = 2;
}
MyObject.prototype = {
   walk: function() {
      // Do stepping
   },
   stop: function() {
      // Stop stepping
   }
};

// Instanciation
var TestMyObject = new MyObject();
TestMyObject.id = 1;
TestMyObject.legs = 3;

However, as I was trying to find a way to do fixed time step timers, I found another way to construct objects by passing in a key value pair object.  That got me thinking that it could be convenient to construct MY objects in a similar fashion.  So is there a problem with creating an object like this:

// Definition
function MyObject(objectSettings) {
   // Essentially acts as default constructor
   objectSettings = objectSettings || {};

   // Without a given parameter, use default
   this.id = objectSettings.id || 0;
   this.legs = objectSettings.legs || 2;

   // Make sure the reference is gone
   objectSettings = null; // Do I even need this?
}
MyObject.prototype = {
   walk: function() {
   },
   stop: function() {
   }
};

// Instanciation

// Use all defaults
var TestMyObject01 = new MyObject();

// Set only id
var TestMyObject02 = new MyObject({
   id: 1
});

// Set everything
var TestMyObject02 = new MyObject({
   id: 1,
   legs: 3
});

My question is, objectSettings doesn't stick around after instantiating the new object right?  Maybe I'm being paranoid.  I'm still trying to wrap my head around closures and initially I had a var inside the definition that referenced the passed objectSettings but I figured that was a bad choice because that would be a private immutable reference to a hunk of memory that was just used to copy into an object and that memory wouldn't go away as long as the created object was around.

I realize it would probably be even easier to pass values in individually to the constructor, but I thought maybe this would be a way to overcome the limitation of not being able to overload a function based on parameter type.  I would pass what I want to set specifically and if it isn't in the passed object (or if there is no passed object at all) the parameter would be set to a default value.  Is there a better way to construct objects in a similar fashion?

I can't find the link to the timer object that got me on this but If I do find it, I'll link it here.

Correct, the lifetime of objectSettings is only for the MyObject function, so you don't need the assignment to null at the end.

Passing in an object to the constructor is a common pattern to set its properties, and if you don't want to add specific assignment statements for each property, you can do something like this:

Utils = {
  // Taken from the ExtJS library
  apply: function(o, c, defaults) {
    if(defaults) {
      Utils.apply(o, defaults);
    }
    if(o && c && typeof c == 'object') {
      for(var p in c) {
        o[p] = c[p];
      }
    }
    return o;
  }
};

function MyObject(cfg) {
  Utils.apply(this, cfg, {
    id: 0,
    legs: 2,
    ...
  });

  // You now have access to this.id, etc.
}


Filtering objects according to required and preferred attributes This my opportunity object which is a part of my angular app.component.ts file. In my template(HTML of this component) I want to loop out different skills backgrounds nationalitiesetc. array using *ngFor*. So I've filtered out these array of objects based on a property option which can have only 2 values either preferred or required. I've implemented such logic inside my TS file using filterByProperty() function as shown below. Please help make this function more simplified and generalized.

opportunity: any = {
 "id": 526,
  "title": "Testing",
  "nationalities": [
    {
      "option": "preferred",
      "id": 479,
      "name": "India",
    }
  ],
  "skills": [
    {
      "option": "required",
      "id": 1153,
      "name": "Adobe Lightroom",
    },
    {
      "option": "preferred",
      "id": 77,
      "name": "Python",
    },
    {
      "option": "preferred",
      "id": 78,
      "name": "Ajax",
    },
    {
      "option": "required",
      "id": 147,
      "name": "3D Max",
    },
    {
      "option": "preferred",
      "id": 175,
      "name": "Aircraft Engine Technology",
    }
  ],
  "backgrounds": [
    {
      "option": "preferred",
      "id": 232,
      "name": "Biology",
    },
    {
      "option": "required",
      "id": 249,
      "name": "International relations",
    }
  ],
  "languages": [
    {
      "option": "required",
      "id": 18,
      "name": "English",
    },
    {
      "option": "preferred",
      "id": 31,
      "name": "Italian",
    }
  ]
}

My app.component.ts file

    import {Component, OnInit, ViewChild} from '@angular/core';

    @Component({
      selector: 'app-root',
      templateUrl: './app.component.html'
    })

    export class AppComponent implements OnInit {

  opportunity: any = {}; // Object value is shown above
  backgrounds: any = {
    preferred: [],
    required: []
  };
  skills: any = {
    preferred: [],
    required: []
  };
  languages: any = {
    preferred: [],
    required: []
  };
  nationalities: any = {
    preferred: [],
    required: []
  };

  ngOnInit() {
      this.filterByProperty();
  }

  filterByProperty() {
    this.skills.preferred = this.opportunity.skills.filter((i) => i.option === 'preferred');
    this.skills.required = this.opportunity.skills.filter((i) => i.option === 'required');
    this.backgrounds.preferred = this.opportunity.backgrounds.filter((i) => i.option === 'preferred');
    this.backgrounds.required = this.opportunity.backgrounds.filter((i) => i.option === 'required');
    this.languages.preferred = this.opportunity.languages.filter((i) => i.option === 'preferred');
    this.languages.required = this.opportunity.languages.filter((i) => i.option === 'required');
    this.nationalities.preferred = this.opportunity.nationalities.filter((i) => i.option === 'preferred');
    this.nationalities.required = this.opportunity.nationalities.filter((i) => i.option === 'required');
  }
}

The most important piece of advice I can offer is get rid of any. By using any you are making Typescript ignore potential errors in your program. opportunity could be described with the following interfaces:

interface Opportunity {
  id: number
  title: string
  nationalities: Criteria[]
  skills: Criteria[]
  backgrounds: Criteria[]
  languages: Criteria[]
}

interface Criteria {
  option: 'preferred' | 'required'
  id: number
  name: string
}

Furthermore, the backgrounds, skills, languages, and nationalities properties should be correctly typed. In this case, I'd just add another interface, SeparatedCriteria isn't really the best name, but you can probably come up with a better one as you are more familiar with what each piece of data represents.

interface SeparatedCriteria {
  preferred: Criteria[]
  required: Criteria[]
}

filterByProperty has a lot of redundancy. At the very least, I would recommend defining isRequired and isPreferred local functions to help cut down on some of the noise.

However, this is still rather noisy especially as the logic is the same for each property - the only thing that changes is the key. With this in mind, we can define an array of keys, ['skills', 'backgrounds', 'languages', 'nationalities'] and assign this[key].preferred and this[key].required.

The trick to making this work correctly is to use both intersection types and index types. It is possible to do this without knowledge of these topics, but the solution will not be type safe as it would be possible to add keys which aren't present in both Opportunity and AppComponent without receiving a compiler error. Here's how I would implement this function. You could now get rid of isRequired and isPreferred as they are only used once, but I believe it makes the logic more clear to leave them as is.

filterByProperty() {
  const isPreferred = ({ option }: Criteria) => option == 'preferred';
  const isRequired = ({ option }: Criteria) => option == 'required';

  const propertyKeys: Array<keyof AppComponent & keyof Opportunity>
    = ['skills', 'backgrounds', 'languages', 'nationalities'];

  for (const key of propertyKeys) {
    this[key].preferred = this.opportunity[key].filter(isPreferred);
    this[key].required = this.opportunity[key].filter(isRequired);
  }
}


Loading YouTube videos inside a bootstrap modal The following code loads a YouTube video inside an iframe, where the tricky part is to sync the onYouTubeIframeAPIReady and on('shown.bs.modal') functions. The code works well in Firefox and decently in Chrome, but in Safari it is quite buggy. Any hints towards improvement would be appreciated.

jQuery( document ).ready(function() {
var tag = document.createElement('script');
tag.src = "https://www.youtube.com/iframe_api";
var firstScriptTag = document.getElementsByTagName('script')[0];
firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);

jQuery("#video_modal").on('shown.bs.modal', function() {
    if(typeof player.playVideo == 'function') {
        player.playVideo();
    } else {
        var fn = function(){
            player.playVideo();
        };
        setTimeout(fn, 200);
    }
});
jQuery("#video_modal").on('hidden.bs.modal', function() {
    player.stopVideo();
});
});

var player;

function onYouTubeIframeAPIReady() {
player = new YT.Player('player', {
    videoId: 'zg8KE6bEP50',
    origin: 'http://rollnstroll.se',
    width: '640',
    height: '390'
});
}

Don't remove the indentation inside of code blocks so that the code is on the same line as the signature. It makes the code harder to read as it's hard to piece together which } goes to which signature.

For example, this:

function onYouTubeIframeAPIReady() {
player = new YT.Player('player', {
    videoId: 'zg8KE6bEP50',
    origin: 'http://rollnstroll.se',
    width: '640',
    height: '390'
});
}

Would become this:

function onYouTubeIframeAPIReady() {
    player = new YT.Player('player', {
        videoId: 'zg8KE6bEP50',
        origin: 'http://rollnstroll.se',
        width: '640',
        height: '390'
    });
}

var tag = document.createElement('script');

The name "tag" is extremely general and gives little aid in defining the use of the variable. Since in the next line you give it a script source, I recommend calling it something that defines the script source.

Here is what I came up with

var ytIframeAPI = document.createElement("script");

Why are you inserting a script into the HTML via JavaScript? Why don't you just put the script tag in your HTML so it is loaded to start?

That way, you don't have to wait for the script to load from the source after the page has loaded; the script will load as the page is loading.

var fn = function(){
    player.playVideo();
};
setTimeout(fn, 200);

Again, like "tag", "fn" means nothing to the variable; I know that it is a function. The variable name does not have to tell me.

You have two options here:

Give it a better name.
Use an anonymous function.

Since the only purpose of that function is to use it in setTimeout, you can just use an anonymous function in place of fn.

Here is what I mean:

} else {
    setTimeout(function() {
        player.playVideo();
    }, 200);
}

See? I didn't even have to give the function a name.

This might just be a typo, but this confuses me:

if(typeof player.playVideo == 'function') {
    player.playVideo();
} else {
    var fn = function(){
        player.playVideo();
    };
    setTimeout(fn, 200);
}

If player.playVideo wasn't a function, it's going to go to else. Then, you call player.playVideo. Well, if it wasn't a function, how are you supposed to call it?


Add classes onclick and then remove them with onfocus event Right now I'm mixing JS and jQuery. I would like to find one unique solution with JS only possibly.

How can I organize the code according to best code practices?

Another thing I need to add is a check at the beginning of the function. If the classes are there nothing should happen, if not there then add them just as below.

function changeClass() {
    document.getElementById("message").className += " dropzone-wrap";
    document.getElementById("dragNdrop").className += " dropzone-content";
}

window.onload = function() {
    document.getElementById("ToggleTxTDrop").addEventListener( 'click' , changeClass );
}

/* lets remove the previous classes when on focus */
$('#message').on('focus', function(){
    $(this).removeClass('dropzone-wrap');
    $(this).parent().removeClass('dropzone-content');
})

Here's a step-by-step conversion from your code to the JS-only code of youmightnotneedjquery.com.

function changeClass() {
    document.getElementById("message").className += " dropzone-wrap";
    document.getElementById("dragNdrop").className += " dropzone-content";
}

Your method here can only ever be used to add "dropzone-wrap" and "dropzone-content". Consider a different approach, where you provide the DOM Element and CSS Classes as parameters.

Consider two such methods - one to add the class (from $.addClass()), and one to remove it ($.removeClass()).

// http://youmightnotneedjquery.com/#add_class
function addClass(el, className) {
    if (el.classList)
      el.classList.add(className);
    else
      el.className += ' ' + className;
}

// http://youmightnotneedjquery.com/#remove_class
function removeClass(el, className) {
    if (el.classList)
      el.classList.remove(className);
    else
      el.className = el.className.replace(new RegExp('(^|\\b)' + className.split(' ').join('|') + '(\\b|$)', 'gi'), ' ');
}

Note that I'd remove the class before adding it, as that code does not protect against long repeated classname strings.

Anyway, let's continue with your code:

window.onload = function() {
    document.getElementById("ToggleTxTDrop").addEventListener( 'click' , changeClass );
}

Using onload works, but it isn't the right way to do it. With jQuery this should be $(window).on('load', function() { ... });... with native Javascript, it isn't much different (.addEventListener).

For the function code itself, create a handler method that uses the addClass() abstraction method to replace your old changeClass() functionality:

function txtDropClicked(evt) {
    addClass(document.getElementById('message'), 'dropzone-wrap');
    addClass(document.getElementById('dragNdrop'), 'dropzone-content');
}

window.addEventListener('load', function() {
    document.getElementById("ToggleTxTDrop").addEventListener( 'click' , txtDropClicked );
});

Now we have your classes being added on click; let's update the onfocus handler in the same fashion:

/* lets remove the previous classes when on focus */
function removeClassesOnFocus(evt) {
    // evt.target is the element that triggered the event. We bound to #message.
    removeClass(evt.target, 'dropzone-wrap');
    removeClass(evt.target.parentNode, 'dropzone-content');
}

// this bit really should wait for `window.load` too.
document.getElementById('message').addEventListener('focus', removeClassesOnFocus);

  If the classes are there nothing should happen

To address that, the removeClassesOnFocus function could be reworked to look up the element if it isn't provided.

function txtDropClicked(evt) {
    removeClassesOnFocus(false);
    addClass(document.getElementById('message'), 'dropzone-wrap');
    addClass(document.getElementById('dragNdrop'), 'dropzone-content');
}

function removeClassesOnFocus(evt) {
    // evt.target is the element that triggered the event. We bound to #message.
    var message = (evt === false) ? document.getElementById('message') : evt.target;
    removeClass(message, 'dropzone-wrap');
    removeClass(message.parentNode, 'dropzone-content');
}

The full converted code:

// http://youmightnotneedjquery.com/#add_class
function addClass(el, className) {
    if (el.classList) { el.classList.add(className); }
    else { el.className += ' ' + className; }
}

// http://youmightnotneedjquery.com/#remove_class
function removeClass(el, className) {
    if (el.classList) { el.classList.remove(className); }
    else { el.className = el.className.replace(new RegExp('(^|\\b)' + className.split(' ').join('|') + '(\\b|$)', 'gi'), ' '); }
}

// When the txtDrop toggle is clicked, add these classes to the message and drag/drop thing.
function txtDropClicked(evt) {
    removeClassesOnFocus(false);
    addClass(document.getElementById('message'), 'dropzone-wrap');
    addClass(document.getElementById('dragNdrop'), 'dropzone-content');
}

// Removes the classes after the message receives focus.
function removeClassesOnFocus(evt) {
    // evt.target is the element that triggered the event. We bound to #message.
    var message = (evt === false) ? document.getElementById('message') : evt.target;
    removeClass(message, 'dropzone-wrap');
    removeClass(message.parentNode, 'dropzone-content');
}

// Attach the click handler to txtDrop toggle when the page loads.
window.addEventListener('load', function() {
    document.getElementById("ToggleTxTDrop").addEventListener( 'click' , txtDropClicked );
    document.getElementById('message').addEventListener('focus', removeClassesOnFocus);
});

It is worth noting that you can use inline functions as handlers, too - and not define txtDropClicked or removeClassesOnFocus. Either way, the point of each method is to perform a single task, and to do that in the most reusable manner possible.


Avoiding verbose compositional class design I've written some code which features Eric Elliot's javascript factory function mixin composition (https://medium.com/javascript-scene/javascript-factory-functions-with-es6-4d224591a8b1), but since there's no object field name associated with the class it doesn't seem like most other forms of composition I see. What would the draw backs be compared the style of composition where each class you compose with is given a field name (I'm calling this named composition)?
let pipe = (...fns) =>x => fns.reduce((acc,fn)=> {
   return fn(acc)
 },x)
let createCanvas= ()=>o=> ({
   ...o,
   makeCanvas(width,height) {
     this.can = document.createElement("canvas")
     this.can.width = width
     this.can.height = height
     this.ctx = this.can.getContext("2d")
   },
   clearCanvas() {
     this.ctx.clearRect(0,0,this.can.width,this.can.height)
   },
   returnCanvas() {
   return this.can
 }
 })
  let drawingLines = () => o => ({
    ...o,
    line(x, y, xx, yy) {
      this.ctx.beginPath();
      this.ctx.moveTo(x, y);
      this.ctx.lineTo(xx, yy);
      this.ctx.stroke();
    }
  });
  let mouseTracker = () => o => ({
    ...o,
    trackCanvas() {
      this.track(this.can);
    },
    track(ele) {
      ele.addEventListener("mousemove", e => {
        this.bb = ele.getBoundingClientRect();
        this.mx = e.clientX - this.bb.left;
        this.my = e.clientY - this.bb.top;
      });
    }
  });
  let rectMethods = () => ({
    makeRect(x, y, w, h) {
      this.ctx.strokeRect(x, y,  w, h);
    },
  });
  let rectMixin = () => o => ({
    ...o,
    ...rectMethods()
  });

  // apparently width gets set automatically, which is pretty nice
  let height = 150;
  let width= 150;
  let firstCanvas = pipe(
    createCanvas(),
    drawingLines(),
    mouseTracker(),
    rectMixin()
  )({});

  firstCanvas.makeCanvas(width, height);
  firstCanvas.trackCanvas();

I'm very interested in avoiding the verbosity of named composition, especially in cases where an existing class has many methods that I would have to rewrite the methods for.
let usefulEventFunctions=()=>({
//...imagine functions named one-five for countings sake...
  evOne() {
},
evTwo() {
},
})
let usefulMathFunctions =()=>({
//...imagine another set of named functions one-five also...
mthOne(){
},
mthTwo(){
}
})
// here's the class composed of the others
let myObject = ()=>({
  math:usefulMathFunctions(),
  events:usefulEventFunctions(),
  // below would be the 10 signatures
evOne() {
   this.events.evOne()
},...
mthTen() {
this.math.mthTen()
}
})

Using the mixin composition I can see how we would avoid this in a language like javascript, but looking over https://stackoverflow.com/questions/49002/prefer-composition-over-inheritance, and Object Composition, it appears other languages only feature named composition, so how is one supposed to avoid all the duplicated methods in choosing composition over inheritance?

Question

*"...so how is one supposed to avoid all the duplicated methods in choosing composition over inheritance?"

I assume you are talking about JavaScript.
It might be a minor point but In JS we use functions, methods are reserved for the engine where the name comes from the language it is written in V8 for example is written in C++
Duplicating functions
There is no need to duplicate functions no matter how you create your object in JavaScript.
I just can not workout why you would want to do
From your bottom snippet

mthTen() {                // WHY??????
    this.math.mthTen()
}

Examining your example via rewrites
Defining the objects
const Events = () => ({evOne() {}, evTwo() {}});
const Maths = () => ({mthOne() {}, mthTwo() {}});

BTW Use literal if you don't need closure
BTW  you would only define objects (using this pattern) via functions if you were closing over some encapsulated data. They are better defined as literals in this case, avoiding the creation of unused closure for each object
const events = {evOne() {}, evTwo() {}};
const maths = {mthOne() {}, mthTwo() {}};

At the object level
If you want the functions to be accessible at the object level you may build it as follow
const createFoo = (math, events) => ({...math, ...events});
const obj = createFoo(math, events);

// Calling
obj.evOne();
obj.mthOne();

At the object property level
To create them as named properties
const createFoo = (math, events) => ({math:{...math}, events:{...events}});
const obj = createFoo(math, events);

// Calling
obj.events.evOne();
obj.math.mthOne();

As static functions
In many cases you would use them as static functions.
// static instance reference. There is only one math and events for many Foos
const Foo = (math, events) => ({math, events});
const obj = Foo(math, events);

// Calling
obj.events.evOne();
obj.math.mthOne();

Or
// Static instance. There is only one instance of math and events referenced by the constructor  Foo
const Foo = Object.assign(() => ({}), {math, events});
const obj = Foo();
// Note access via the instantiation function  Foo
Foo.events.evOne();
Foo.math.mthOne();
obj.event.evOne(); // will throw as it does not exist.

Or
// Static instance functions. There is only one instance of math and events
// referenced via functions of the constructor  Foo
const Foo = Object.assign(() => ({}), {...math, ...events});
const obj = Foo();

// Note access via the instantiation function  Foo
Foo.evOne();
Foo.mthOne();
obj.evOne(); // will throw as it does not exist.

Note there is no formal way to define static functions its only static by how it is used and instanced (Via the class syntax static is just syntactical sugar for (assign to constructor, rather than prototype))
If you must duplicate
Warning Don't do this at home, it is only as an example. Doing this in any code can result in severe confusion and eventual application death.
If you really wanted to have a second copy of each function. eg obj.evOne and obj.event.evOne You can automate the function creation, binding the copied function references to the object you want as this

const events = {evOne() {}, evTwo() {}};
const logger = {
    logA() { console.log("A" + this.count++)},
    logB() { console.log("B" + this.count++)},
    count: 0
};

const addFunctions = (obj, ...instances) => {
    for (const instance of instances) {
        for (const [name, func] of Object.entries(instance)) {
            func instanceof Function  && (obj[name] = func.bind(instance));
        }
    }
    return obj;
}

const createFoo = (a, b) => addFunctions({logger: a, events: b}, a, b);
const foo = createFoo({...logger}, {...events});

foo.logger.logA();  // should be A0
foo.logA();         // should be A1
foo.logger.logB();  // should be B2
foo.logB();         // should be B3

Question

"What would the draw backs be compared the style of composition where each class you compose with is given a field name (I'm calling this named composition)?"

This question is way too subjective to answer in any death.
There are drawbacks to every pattern, how the drawbacks effect the code quality is very dependent on the coders skill and level of experience.
If you are comfortable with a pattern and have the experience your proficiency outweighs any pattern's drawbacks.
The biggest drawback to any pattern is how experience a coder is in using it.
To be a well rounded coder you must gain proficiency in all the patterns.


Display all solutions for NQueens In my previous NQueens Code Review question, I received feedback saying that I should write a program that would find all solutions. This revised program should be more general than before.

import java.util.*;

public class RedoNQueens {
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        System.out.print("Enter number of queens: ");
        int nQueens = input.nextInt();
        //Create board and clear it
        int[] board = new int[nQueens];
        for (int i = 0; i < board.length; i++) {
            board[i] = -1;
        }

        //Place the first piece to get started
        int row = 0;
        board[row] = 0;
        row++;
        int column = 0;
        long solutions = 0;
        while (row >= 0) {
            if (isValid(row, column, board)) {
                board[row] = column;
                row++;
                column = 0;

                //Board completed?
                if (row >= board.length) {
                    solutions++;
                    System.out.println("Solution " + solutions);
                    printBoard(board);
                    row--;
                    column = board[row] + 1;
                }
            }
            else {
                //If current column is not valid
                column++;
                if (column >= board.length) {
                    board[row] = -1;
                    row--;
                    if (row >= 0)
                        column = board[row] + 1;
                }
            }
        }
        System.out.println("Number of solutions for " + nQueens + " queens is " + solutions);
    }

    public static boolean isValid(int row, int column, int[] board) {
        if (column >= board.length)
            return false;

        for (int i = 0; i < row; i++) {
            //Check if column appears in any other row
            if (board[i] == column)
                return false;

            //Check both diagnals
            if (Math.abs(row - i) == Math.abs(column - board[i]))
                return false;
        }

        return true;
    }

    public static void printBoard(int[] board) {
        for (int i = 0; i < board.length; i++) {
            for (int j = 0; j < board.length; j++) {
                if (board[i] == j)
                    System.out.print("|Q");
                else
                    System.out.print("| ");
            }
            System.out.println("|");
        }
    }
}

I'm not sure what you're looking for specifically in trying to revise your program past this point. So, here's a handful of general things I would change:

Remove static modifier from your code. This can be done by creating an NQueens class, that maybe takes the N as a parameter to it's constructor, then can be run by calling a "run" method.
Use curly braces on all your conditional statements. You're less likely to break code down the road that way.
See all those comments you have? Instead of commenting on little sections of that, you could increase your code's readability by making those their own methods.

Example: In the following code, maybe make a method that explains what you're doing here, such as resetPosition or something.

    //If current column is not valid
    column++;
    if (column >= board.length) {
        board[row] = -1;
        row--;
        if (row >= 0)
            column = board[row] + 1;
    }

I would suggest trying to better name your iterators as well. i and j are just fine, but if you're actually iterating through something that has a name, such as row, column, etc. go ahead and use that. Again, easier to read and follow the algorithm.
Similarly, row, column aren't totally obvious in how they're used. Would they be better represented as currentRow or currentColumn? Since you're using these as nodes to find new solutions from, it might makes sense to have a more descriptive name.
isValid; what exactly is valid? Is that a valid representation of the board, a valid solution, or only part of the solution?
Lastly, why isn't the board represented as a 2D array? You received this as feedback last time as well, but I second it. A game board is represented in 2-dimensional space, x and y. The same way a cube would be in 3-dimensional space and therefor hopefully be represented as a 3-dimensional array. Trying to represent objects outside their native dimension takes some time to understand. I would avoid this at all costs unless there are serious performance gains to be made.

I think the point I'm getting at here is that: Ideally, I shouldn't have to know anything about what the NQueens problem is in order to follow your code without much issue.


Find items in collection one that are the greatest value less than an item in collection two This is hard to explain.  I wanted to get all of the items in collection one (ints in the example below) that are the highest values that are lower than any of the items in collection two (otherInts).  In the example below, only 2 and 8 should be returned from ints.  1 doesn't qualify because it is lower than 3 (in otherInts), but 2 is higher than 1, but still lower than 3.  2 does qualify because there is nothing higher than it that is lower than 3.  5 does not qualify for the same reason as 1, namely, 8 is higher than it, but also lower than 9 (in otherInts).

I've written the following linq to accomplish this.  I'd like to stay constrained to linq as opposed to a standard forEach since this is a sub-linq statement.  Is there a better linq way to accomplish this?

var ints = new List<int>{1,2,5,8};
var otherInts = new List<int>{3,4,9};

var intsImmediatelyLessThanOrEqualToOtherInts = from i in ints
    let firstGreaterOtherInt = otherInts.First(o => o > i)
    let intBetweenThisAndOther = from i2 in ints
        where i2 > i && i2 < firstGreaterOtherInt
        select i2
    select intBetweenThisAndOther.Count() > 0 ? null as object : i;

intsImmediatelyLessThanOrEqualToOtherInts.Where(i => i != null).ToList().ForEach(Console.WriteLine);

Here's a working fiddle if you want to test other cases.

To me there can only be two such numbers in the first set an they can be selected with these two queries:

var set1MaxLessThenSet1Min = set1.TakeWhile(i => i < set2 .Min()).LastOrDefault();
var set1MaxLessThenSet2Min = set1.TakeWhile(i => i < set2 .Max()).LastOrDefault();

this means that for those two sets:

var set1 = new List<int> { 1, 5, 6, 7, 9 };
var set2 = new List<int> { 3, 4, 9, 10, 11 };

those numbers are 1 and 9 whereas your query finds also the 7 which is not at the same time the highest number in set1 and lower then the highest in set2.

Both queries are for ordered sets. If they are not ordered the Where clause should be used.


Finding Local Maxima The following function finds all local maxima.

A local maxima is an element of a list such that it's greater than either of its neighbor elements. An element with 1 neighbor is not a local maximum.

localMaxima :: [Integer] -> [Integer]
localMaxima (x:y:z:zs) = if (y > x && y > z) then y : localMaxima (y:z:zs)
                         else localMaxima (y:z:zs)
localMaxima _          = []

Please critique it.

Examples:

ghci> localMaxima [1,3,2,55,2]
[3,55]
ghci> localMaxima [1,3,2,55,1000000000]
[3]

You have redundant parentheses here: (y > x && y > z). You can also make your type more generic: localMaxima :: Ord a => [a] -> [a].

I think it looks a bit cleaner using an as-pattern and guard clauses. This way you don't need zs and can just write _.

localMaxima (x:rest@(y:z:_))
  | y > x && y > z = y : localMaxima rest
  | otherwise      = localMaxima rest
localMaxima _ = []

In fact, if you know that y > z then you don't need to check if z is a local maximum.


Factory hierarchy for creating GoogleMap and its elements The purpose of this code is to create a GoogleMap using PHP, add markers to it according to Place objects from database, add infoWindows for these markers. While reading about Factory pattern I came up with the following solution. What are the parts that can be improved. Thank you.

Factory for creating markers:

class MarkerFactory
{
    public function create(Coordinate $position, array $options = [])
    {
        $marker = new Marker($position);
        $marker->setOptions($options);

        return $marker;
    }
}

Factory for creating infoWindows. Isn't it violating any principles because of injected dependency?

class InfoWindowFactory
{
    private $twig;

    public function __construct(\Twig_Environment $twig)
    {
        $this->twig = $twig;
    }

    public function create(Place $place, $editable = false)
    {
        return new InfoWindow($this->renderWindowContent($place, $editable));
    }

    protected function renderWindowContent(Place $place, $editable)
    {
        if ($editable) {
            $template = 'editable_infowindow.html.twig';
        } else {
            $template = 'static_infowindow.html.twig';
        }

        return $this->twig->render($template, ['place' => $place]);
    }
}

MapFactory that uses MarkerFactory and InfoWindowFactory:

class MapFactory
{
    private $markerFactory;

    private $infoWindowFactory;

    public function __construct(MarkerFactory $markerFactory, InfoWindowFactory $infoWindowFactory)
    {
        $this->markerFactory = $markerFactory;
        $this->infoWindowFactory = $infoWindowFactory;
    }

    public function create(Coordinate $mapCenter, array $places = [], array $mapOptions = [], $isEditableMap = false)
    {
        $map = new Map();

        $map->setCenter($mapCenter);
        $map->setMapOptions($mapOptions);

        $markers = [];

        foreach ($places as $place) {
            $position = new Coordinate($place->getLatitude(), $place->getLongitude());

            $marker = $this->markerFactory->create($position);
            $infoWindow = $this->infoWindowFactory->create($place, $isEditableMap);

            $marker->setInfoWindow($infoWindow);

            $markers[] = $marker;
        }

        $map->getOverlayManager()->setMarkers($markers);

        return $map;
    }
}

And the code example of usage:

$markerFactory = new MarkerFactory();
$infoWindowFactory = new InfoWindowFactory($this->get('twig'));
$mapFactory = new MapFactory($markerFactory, $infoWindowFactory);

$center = new Coordinate(55.117100, 23.940332);
$options = ['zoom' => 7];
$map = $mapFactory->create($center, $places, $options);

I think your conception of the factory pattern is a little off.  Throughout your classes, you seem to want to deal with concrete factory objects, which doesn't seem to make sense.  Typically, a factory would be invoked statically and would not store local references (i.e. in properties) to the dependencies it is acting upon.

Your MarkerFactory class is close, just needing to have create() method defined as static and possibly defining the class itself as abstract since there would not seem to be any reason to ever instantiate it.

But getting into your other classes, there seem to be issues with where logic lies.  The InfoWindowFactory probably should not have a method such as renderWindowContent() on it.  That would seem to be a method that should perhaps exist on the InfoWindow itself.  Now, if the factory needs to call that method on a concrete window object to get it set up properly before returning it, that might be OK (though I might suggest the InfoWindow constructor is changed to trigger this action).

I would like to see InfoWindowFactory be something like:

namespace YourNamespace;

abstract class InfoWindowFactory
{
    public static final function create(
        Place $place,
        Twig_Environment $twig,
        $template
    ) {
        return new InfoWindow($place, $twig, $template);
    }
}

Note that we are pushing the render operation down into the InfoWindow object where it probably belongs.  Also note that we have removed the hard-coded template names from this class.  Maybe they truly belong here, in which case maybe they should be class constants rather than buried in a method. But, depending on how you are using these InfoWindows, perhaps this should be passed through as configuration.

My comments on MapFactory would be similar to those for the InfoWindowFactory.  Namely, I would rather see a class like structured like this:

namespace YourNamespace;

use MarkerFactory;
use InfowindowFactory;

abstract class MapFactory
{

    public static final function create(Coordinate $mapCenter, array $places = [], array $mapOptions = [], $isEditableMap = false)
    {
        $map = new Map();

        $map->setCenter($mapCenter);
        $map->setMapOptions($mapOptions);

        $markers = [];

        foreach ($places as $place) {
            $position = new Coordinate($place->getLatitude(), $place->getLongitude());

            $marker = MarkerFactory::create($position);
            $infoWindow = InfoWindowFactory::create($place, $isEditableMap);

            $marker->setInfoWindow($infoWindow);

            $markers[] = $marker;
        }

        $map->getOverlayManager()->setMarkers($markers);

        return $map;
    }
}

A comment about validation...

You are doing a good job of trying to enforce typehints on your methods, however I do wonder if it might make sense for you to have classes defined which deal with say a collection of Place objects.  These seem to be critical dependencies that can only be enforced to be an array via type-hinting, meaning that if you truly wanted to be able to validate that you have valid place objects in your array, you might have to go into undesirable approach of inspecting each array item to validate it.  If instead of passing simple array, you passed PlaceCollection or similar, you could be ensured that everything in that collection is a valid Place object.   Certainly, this adds yet another bit of code to the overall system, so really only you can determine if it is worthwhile to do this depending on how you are planning to use this code.  But it is something to consider when these are key items you are working with (vs. say optional configurations or similar).

Do you need any error or exception handling here?  Typically one of the things a factory would do is raise an exception if the dependencies that are passed are not valid (typehinting can do a lot of this for you) or to handle any underyling exceptions that might happen within method execution (i.e. can instantiation of a Map, InfoWindow, etc. object throw an exception?).  I don't really see any consideration here in your code for error or edge conditions.  It seems as if this is considering happy path only.


Handling user input - follow-up Original code: Getting name from user, and handling any errors in C

I fixed the bugs. Is there anything else to consider?

#include "stdafx.h"
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdbool.h>

#define BUFFSIZE 10

/*Checks to see if name contains actual characters and not just whitespace*/
bool hasAlpha(unsigned const char * name, size_t buffersize)
{
    for (size_t index = 0; index < buffersize && name[0] != '\0' ; ++index) {
        if (isalpha(name[index]))
            return true;
    }

    return false;
}

/* @param[nlcspan], Used to determine if newline char was in C string or not */
/* @param[incomplete], If true, new line character is still in the buffer i.e. need to clear buffer*/

void retrieveName(unsigned char* name, size_t buffersize)
{
    for (;;) {
        if (fgets(name, buffersize, stdin) != NULL) {
            size_t nlcspn = strcspn(name, "\n");
            bool incomplete = name[nlcspn] == 0;
            name[nlcspn] = 0;
            if (incomplete) {
                int bufferclear;
                while ((bufferclear = getchar()) != '\n' && bufferclear != EOF);
            }
            if (name[0] == '\0' || !hasAlpha(name, buffersize)) {
                printf("Invalid name entered please try again...\n"
                       "USER NOTE: name cannot be empty and must contain at least one char\n");
            }
            else {
                return;
            }
        }
        else
            printf("Memory could not be allocated please try again...\n");
    }
}

int main(void)
{
    size_t len = BUFFSIZE;
    unsigned char name[BUFFSIZE];

    printf("Enter Name: ");
    retrieveName(name, len);
    printf("Your name is: %s\n", name);

    printf("Enter Name: ");
    retrieveName(name, len);
    printf("Your name is: %s\n", name);

    printf("Press enter to continue...\n");
    getchar();

    return 0;
}

BUGS
The function retrieveName() is doing the input of the name twice.

        if (fgets(name, buffersize, stdin) != NULL) {
            size_t nlcspn = strcspn(name, "\n");
            bool incomplete = name[nlcspn] == 0;
            name[nlcspn] = 0;
            if (incomplete) {                    int bufferclear;
                while ((bufferclear = getchar()) != '\n' && bufferclear != EOF);
            }
            if (name[0] == '\0' || !hasAlpha(name, buffersize)) {
                printf("Invalid name entered please try again...\n"
                       "USER NOTE: name cannot be empty and must contain at least one char\n");
            }
            else {
                return;
            }

Both fgets() and getchar() are getting input from stdin. The search you are
performing for end of line is failing because bufferclear should be EOF on the
first execution of the loop. What you should be doing is a search for end of line
on name.

Keep it Simple
Since you are reading from stdin, use gets() rather than fgets(), gets() has
the same semantics as fgets() but only reads from stdin. If you want
retrieveName() to be able to read from any file you should add another parameter
this is either the file name or a FILE pointer. If it is the file name than
retrieveName() will need to perform a fopen().

There is a software engineering/programming principle call KISS, or Keep it Simple.
_principle
The simpler you make the code the easier it is to write, maintain and debug.

Alignment
If name is going to be used in a struct at some point in the future, you should
consider the alignment of the fields in the struct to make the most efficient use
of memory. Computers implement strings in words of memory. A word is some power
of two bytes long, in the past this has been either 2 or 4 bytes, most computers
and compilers are going to use 8 bytes these days in a 64 bit architecture.
Therefore BUFFSIZE should be some multiple of 8 (8, 16, 24, 32). I'd suggest
as a minimum 16 because 8 may be too small.

Use Functions or Loops for Repetitive Code
In software engineering there is a programming principle called Don't Repeat Yourself.
Any time you have repeating code such as :

    printf("Enter Name: ");
    retrieveName(name, len);
    printf("Your name is: %s\n", name);

in main() you should consider either creating a loop or writing a function.
Generally I would recommend writing a function, but in this case a loop will
do just as well.


Intersections of two lists Given two python lists a and b. These lists may intersect (It is possible that lists intersect more than once). An example:

a = [1,2,5,7,6,9,1,3,1,2,6,6,3,1,2,6,1,3]
b = [3,1,2]

There are 4 intersections:

  [1,2,5,7,6,9,1,3,1,2,6,6,3,1,2,6,1,3]
[3,1,2]         [3,1,2]   [3,1,2]   [3,1,2]

For each intersection I can write equality: u + a + v == w + b + t

I need to write a function that returns lists u, v, w, t. For this example these lists are:

1. u = [3], v = []
   w = [],  t = [5, 7, 6, 9, 1, 3, 1, 2, 6, 6, 3, 1, 2, 6, 1, 3]

2. u = [],  v = []
   w = [1, 2, 5, 7, 6, 9, 1], t = [6, 6, 3, 1, 2, 6, 1, 3]

3. u = [],  v = []
   w = [1, 2, 5, 7, 6, 9, 1, 3, 1, 2, 6, 6], t = [6, 1, 3]

4. u = [], v = [1, 2]
   w = [1, 2, 5, 7, 6, 9, 1, 3, 1, 2, 6, 6, 3, 1, 2, 6, 1], t = []

The following is a function I implemented (there is precondition: len(lhs) >= len(rhs)):

def get_additions(lhs, rhs):
    results = []
    szl = len(lhs)
    szr = len(rhs)
    i = 1 - szr
    while i < szl:
        a = max(0, i)
        b = max(0, -i)
        c = min(i + szr, szl)
        d = min(szr, szl - i)

        if (lhs[a:c] == rhs[b:d]):
            u = rhs[0:b]
            v = rhs[d:szr]
            w = lhs[0:a]
            t = lhs[c:szl]
            results.append((u, v, w, t))
        i += 1
    return results

How to make it better?

at line if (lhs[a:c] == rhs[b:d]): parentheses are redundant
you don't need to return whole list, you can return only iterator (remove results variable, change line results.append((u, v, w, t)) into yield u, v, w, t). After it you can always use this as list if needed (list(get_additions(a, b))) but usually you need only iterator.
your variable i change from <1 - szr .. szl). You can use for i in xrange(1 - szr, szl): to do this
at your description i can read about conditions u + a + v == w + b + t and len(lhs) >= len(rhs). You can write it in your code as assert
at your description variables a, b means something else than in your code. It's not good idea. Your also use various conventions of variable naming (e.g. szr as size of right list and b as begining index of right list). You should define variables in the same way.
personally i often remove variables like szr because len() of list is quite fast. It's your choice (you get less local variables but more len() calls).
you always cut b[3:len(b)] into b[3:] and b[0:3] into b[:3]

After these changes your code could looks like

def get_additions(a, b):
    assert len(a) >= len(b)
    for i in xrange(1 - len(b), len(a)):
        a_left = max(0, i)
        b_left = max(0, -i)
        a_right = min(i + len(b), len(a))
        b_right = min(len(b), len(a) - i)

        if a[a_left:a_right] == b[b_left:b_right]:
            u = b[:b_left]
            v = b[b_right:]
            w = a[:a_left]
            t = a[a_right:]
            assert u + a + v == w + b + t
            yield u, v, w, t

if __name__ == "__main__":
    a = [1, 2, 5, 7, 6, 9, 1, 3, 1, 2, 6, 6, 3, 1, 2, 6, 1, 3]
    b = [3, 1, 2]
    for u, v, w, t in get_additions(a, b):
        print "u = %s, v = %s\nw = %s, t = %s\n" % (u, v, w, t)


Find a powerset of a set in C The code below is an implementation of purely recursive solution to find a powerset of any given set. The input set is  a string, and the output - array of strings. Both of them are wrapped into a structure for convenience. The underlying algorithm is pretty common one. The recursive function executes exactly 2^n - 1 times.
I've intentionally removed all memory allocation error checking to make the snippet shorter.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct set {
    char* data;
    int size;
} Set;

typedef struct pset {
    char **data;
    int size;
} PSet;

Set *initSet(char *str) {
    Set *set;
    set = malloc(sizeof(Set));

    set->data = calloc(strlen(str) + 1, 1);
    strcpy(set->data, str);
    set->size = strlen(str);

    return set;
}

PSet *initPSet(const Set* sset) {
    // wrapper for recursive function getPSet
    void getPSet(PSet *pset, const Set *inputStr, int buffer, int index);

    PSet *pset = malloc(sizeof(PSet));

    pset->data = malloc (sizeof(char *) * (1 << sset->size));

    // empty set is a subset of any set
    pset->data[pset->size] = calloc(2, 1);

    strcpy(pset->data[pset->size], "");
    pset->size = 1;

    // in case the input set is empty return a set with just one element
    if (sset->size != 0)
        getPSet(pset, sset, 0, 0);

    return pset;
}

void getPSet(PSet *pset, const Set *set, int buffer, int index) {
    //allocating place for a new subset
    pset->data[pset->size] = calloc(strlen(pset->data[buffer]) + 2, sizeof(char));

    strcpy(pset->data[pset->size], pset->data[buffer]);
    pset->data[pset->size][strlen(pset->data[buffer])] = set->data[index];
    pset->size++;

    // local variable pos keeps track of a position of a prefix stored in pset for future recurrent calls
    int pos = pset->size - 1;

    index++;

    if (index >= set->size) return;

    getPSet(pset, set, buffer, index);
    getPSet(pset, set, pos, index);
}

int main () {

    Set *sset = initSet("abcd");
    PSet *pset;

    pset = initPSet(sset);

    for (int i = 0; i < pset->size; ++i) {
        printf("{%s}\n", pset->data[i]);
    }

    return 0;
}

Just a review of a small portion of code:
PSet *pset = malloc(sizeof(PSet));
pset->data = malloc (sizeof(char *) * (1 << sset->size));
pset->data[pset->size] = calloc(2, 1);

pset->size is not yet assigned a value. pset->data[pset->size] is a Bug.  @1201ProgramAlarm
sizeof(char *) --> Is that the correct size?  I need to look for the declarations of pset up some lines and then the definition of PSet some 20 lines upstream.
Consider below instead - no need to check if the correct type.  The size is correct by inspection of this line alone.
pset->data = malloc(sizeof pset->data[0] * (1 << sset->size));

Later code uses int for indexing and buffer size, yet could have used unsigned and incur no additional cost in performance.  To support unsigned, add u:
pset->data = malloc(sizeof pset->data[0] * (1u << sset->size));

Redundant string length calculation
Compiler may not optimize the 2 strlen() calls into 1.  Recommend to do so by direct code.
//pset->data[pset->size] = calloc(strlen(pset->data[buffer]) + 2, sizeof(char));
//strcpy(pset->data[pset->size], pset->data[buffer]);
//pset->data[pset->size][strlen(pset->data[buffer])] = set->data[index];

size_t len = strlen(pset->data[buffer];
pset->data[pset->size] = calloc(len + 2, sizeof(char));
strcpy(pset->data[pset->size], pset->data[buffer]);  // or memcpy()
pset->data[pset->size][len] = set->data[index];  // re-use `len`

Use const for unchanged refenced data
It better conveys code's intent, allows for some optimizations and greater functionally usage.
// Set *initSet(char *str) {
Set *initSet(const char *str) {

Missing freeing of memory
For general usage of these routines, de-allocation routines are needed like uninitSet() and uninitPSet().


Set representation allowing duplicates From SICP:

  Exercise 2.60.  We specified that a
  set would be represented as a list
  with no duplicates. Now suppose we
  allow duplicates. For instance, the
  set {1,2,3} could be represented as
  the list (2 3 2 1 3 2 2). Design
  procedures element-of-set?,
  adjoin-set, union-set, and
  intersection-set that operate on this
  representation. How does the
  efficiency of each compare with the
  corresponding procedure for the
  non-duplicate representation? Are
  there applications for which you would
  use this representation in preference
  to the non-duplicate one?

I wrote the following solution (some parts came from the book):

(define (element-of-set? x set)
  (cond ((null? set) false)
        ((equal? x (car set)) true)
        (else (element-of-set? x (cdr set)))))

(define adjoin-set cons)

(define (intersection-set set1 set2)
  (cond ((or (null? set1) (null? set2)) '())
        ((element-of-set? (car set1) set2)
         (cons (car set1)
               (intersection-set (cdr set1) set2)))
        (else (intersection-set (cdr set1) set2))))

(define (union-set set1 set2)
  (cond ((null? set1) set2)
        ((null? set2) set1)
        (else (cons (car set1) (union-set (cdr set1) set2)))))1)

Just as adjoin-set simply became cons, union-set can be defined as:

(define union-set append)


FizzBuzz from a file This was found on CodeEval.

Challenge: Write a program that prints out the final series of numbers where those divisible by X, Y and both are replaced by “F” for fizz, “B” for buzz and “FB” for fizz buzz

Specifications: Your program should accept a file as its first argument. The file contains multiple separated lines; each line contains 3 numbers that are space delimited. The first number is the first divider (X), the second number is the second divider (Y), and the third number is how far you should count (N). You may assume that the input file is formatted correctly and the numbers are valid positive integers. your output should print out one line per set. Ensure that there are no trailing empty spaces in each line you print.

Sample Input:

  3 5 10
  2 7 15

Sample Output:

  1 2 F 4 B F 7 8 F B
  1 F 3 F 5 F B F 9 F 11 F 13 FB 15

My Implementation:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;

public class FizzBuzz {
    public static void main(String[] args) throws FileNotFoundException {
        File file = new File(args[0]);
        Scanner fileScanner = new Scanner(file);
        while (fileScanner.hasNextLine()) {
            printFizzBuzz(fileScanner.nextLine());
        }
    }

    public static void printFizzBuzz(String line) {
        int fizz = Integer.parseInt(line.split(" ")[0]);
        int buzz = Integer.parseInt(line.split(" ")[1]);
        int limit = Integer.parseInt(line.split(" ")[2]);

        StringBuilder sb = new StringBuilder();

        for (int i = 1; i <= limit; i++) {
            if (i % fizz == 0) { sb.append("F"); }
            if (i % buzz == 0) { sb.append("B"); }
            else { if (i % fizz != 0) { sb.append(i); } }
            if (i < limit) { sb.append(" "); }
        }
        System.out.println(sb.toString());
    }
}

If you extract the logic that builds the output string to its own method buildFizzBuzz,
you'll get something testable:

public static String buildFizzBuzz(int fizz, int buzz, int limit) {
    StringBuilder sb = new StringBuilder();
    // ...
    return sb.toString();
}

public static void printFizzBuzz(String line) {
    int fizz = Integer.parseInt(line.split(" ")[0]);
    int buzz = Integer.parseInt(line.split(" ")[1]);
    int limit = Integer.parseInt(line.split(" ")[2]);

    System.out.println(buildFizzBuzz(fizz, buzz, limit));
}

And you can add some unit tests, for example:

@Test
public void test_3_5_20() {
    assertEquals("1 2 F 4 B F 7 8 F B 11 F 13 14 FB 16 17 F 19 B", buildFizzBuzz(3, 5, 20));
}

@Test
public void test_2_7_15() {
    assertEquals("1 F 3 F 5 F B F 9 F 11 F 13 FB 15", buildFizzBuzz(2, 7, 15));
}

Having these unit test, which pass at the moment,
now we can refactor a bit safely, knowing that if anything breaks, the tests will break.

It's not great to have a check on the limit twice in the loop,
in the loop condition and also in the loop body.
It would be better to eliminate the condition from the body,
always appending ' ', and cutting off the space at the end.

It's not great to have a check on i % fizz twice.
It would be better to use a boolean to track if fizz or buzz were already appended.

Putting these two points together, the implementation becomes:

public static String buildFizzBuzz(int fizz, int buzz, int limit) {
    StringBuilder sb = new StringBuilder();

    for (int i = 1; i <= limit; i++) {
        boolean shouldAppendNum = true;
        if (i % fizz == 0) {
            sb.append("F");
            shouldAppendNum = false;
        }
        if (i % buzz == 0) {
            sb.append("B");
            shouldAppendNum = false;
        }
        if (shouldAppendNum) {
            sb.append(i);
        }
        sb.append(" ");
    }
    sb.setLength(sb.length() - 1);
    return sb.toString();
}


Pythonic way to select element in a list window Here is a list where I would like to keep only the 'windows' where there are a group of number higher than 3, but if in any case inside this group you find a smaller number, you keep it.

Moreover, if there is an isolated high number (between zeros for example), we need to delete it.

l = [3.5, 0, 0, 0.5, 4, 10, 20, 3, 20, 10, 2, 0, 0, 3.5, 0, 2, 18, 15, 2, 14, 2, 0]

and the expected result :

[4, 10, 20, 3, 20, 10, 18, 15, 2, 14]

This program does it but I feel this it not very pythonic, could you think of any other way ?

l = [3.5, 0, 0, 0.5, 4, 10, 20, 3, 20, 10, 2, 0, 0, 3.5, 0, 2, 18, 15, 2, 14, 2, 0]

new_l = []
index = []
seuil = 3

for i, elt in enumerate(l):
    if elt > seuil:
        if (l[i-1] and l[i+1]) > 1:
            new_l.append(elt)
            index.append(i)
        else:
            pass
    else:
        try:
            if (l[i-1] and l[i+1]) > seuil:
                new_l.append(elt)
                index.append(i)
        except IndexError:
            pass
print index
print new_l

Write functions, improvements can range from faster code to the ability to reliably time or profile your code.
You don't need to use else if you're just using pass in that block.
Your code is WET, so it's not DRY. Instead of duplicating the code, you could instead assign to a variable in the if and else, to check against.
You may want to get into a habit of using if __name__ == '__main__':. In short, if you import the file it won't run the code.

This can get you:

def rename_me(l, seuil):
    new_l = []
    index = []
    for i, elt in enumerate(l):
        bound = 1 if elt > seuil else seuil
        try:
            if (l[i-1] and l[i+1]) > bound:
                new_l.append(elt)
                index.append(i)
        except IndexError:
            pass
    return index, new_l

A couple more things:

I think you have a bug, (l[i-1] and l[i+1]) > bound probably isn't doing what you think it is. It's only checking if l[i+1] is greater than bound. As 1 and 2 == 2 and 0 and 2 == 0 are both true. Instead you may want to use l[i-1] > bound < l[i+1].
I don't see why you'd need the indexes and the elements from the original list, and so you could instead just return one or the other.
You can change the function to be a generator function, as this reduces memory usage.
You could use a modified itertools.pairwise, to remove the IndexError.

from itertools import tee

def rename_me(l, seuil):
    a, b, c = tee(l, 3)
    next(b, None)
    next(c, None)
    next(c, None)
    for index, (i, j, k) in enumerate(zip(a, b, c), 1):
        if i > (1 if j > seuil else seuil) < k:
            yield index


Inner Join only if satisfies If condition I'm looking for code review, and optimizations.

I have two tables ItemCategory and ShopItemCategory

ItemCategory

--------------------------------------------------
| No | ItemCategoryId  |      CategoryName       |
--------------------------------------------------
| 1  |    1233         |      Cement             |
--------------------------------------------------
| 2  |    1200         |      Drinks             |
--------------------------------------------------

ShopItemCategory

--------------------------------------------------
| No | ItemCategoryId  |        ShopId           |
--------------------------------------------------
| 1  |    1233         |           25            |
--------------------------------------------------
| 2  |    1200         |           32            |
--------------------------------------------------

I have an sp (stored procedure) to get ItemCategory list for dropdown,

CREATE PROCEDURE [dbo].[spGetItemCategory]

AS
BEGIN

    SET NOCOUNT ON;

    SELECT ItemCategoryId ,CategoryName
    FROM ItemCategory

End

This one works fine, and now I have new requirement. I need to send ShopId to the sp and return ItemCategory list that satisfies the condition, and if the receiving shopid equals 0 then I need to return all the ItemCategory.

My sp :

CREATE PROCEDURE [dbo].[spGetItemCategory]
(
    @ShopID INT
)

AS
BEGIN

     SET NOCOUNT ON;

     if @ShopID <> 0
         BEGIN

              SELECT ItemCategory.ItemCategoryId,CategoryName
              FROM ItemCategory
              inner join ShopItemCategory on
                     ShopItemCategory.ShopId = @ShopID AND
                     ShopItemCategory.ItemCategoryId = ItemCategory.ItemCategoryId

          END
     else
     BEGIN

          SELECT  ItemCategoryId,CategoryName
          FROM aItemCategory

     END
 END

This works well in my code. But in the two conditions only the inner join part is different, So is there a way to reduce the code and improve performance?

I got other stored procedures with larger codes in if conditions, so this method will make it bigger and bigger. Is there any alternate way to do this? or to simplify this?

I did something like this

CREATE PROCEDURE [dbo].[spGetItemCategory] (@ShopID INT)
AS
BEGIN
    SET NOCOUNT ON;

    SELECT ItemCategory.ItemCategoryID
        ,CategoryName
    FROM ItemCategory
    INNER JOIN ShopitemCategory ON (
            ShopitemCategory.shopid = @ShopID
            AND ShopitemCategory.itemcategoryid = ItemCategory.ItemCategoryID
            )
        OR (@ShopID = 0)
END


Candie distribution algorithm I am studying algorithms and I solved this problem under Dynamic Programming section:

  Alice is a kindergarten teacher. She wants to give some candies to the
  children in her class.  All the children sit in a line ( their
  positions are fixed), and each  of them has a rating score according
  to his or her performance in the class.  Alice wants to give at least
  1 candy to each child. If two children sit next to each other, then
  the one with the higher rating must get more candies. Alice wants to
  save money, so she needs to minimize the total number of candies given
  to the children.

  Input Format

  The first line of the input is an integer N, the number of children in
  Alice's class. Each of the following N lines contains an integer that
  indicates the rating of each child.

Code:

import java.io.*;
import java.util.*;

public class Candie{

 static int candieCalculator(int[] numbers) {
    int len = numbers.length;
    int result=0;

    if (len==1) {
        return 1;
    }

    for (int i=0; i<len; i++) {
        result++;
        if (i==0) {
            if (numbers[i] > numbers[i+1]) {
                result++;
            }
        }
        else if (i==len-1) {
            if (numbers[i] > numbers[i-1]) {
                result++;
            }
        }
        else {

            if (numbers[i] > numbers[i+1]) {
                result++;
            }

            if (numbers[i] > numbers[i-1]) {
                result++;
            }
        }
    }

    return result;
 }

 public static void main(String[] args) {

    Scanner in = new Scanner(System.in);
    int n = in.nextInt();
    int[] numbers = new int[n];

    for(int i=0; i < n; i++) {
        numbers[i] = in.nextInt();
    }
    System.out.println(candieCalculator(numbers));

 }
}

Sample input:

  3  1  2  2

Sample output:

4

What do you think? Also this solution is considered as DP? It doesn't look DP to me.

Imports

import java.io.*;
import java.util.*;

It's more common to import just what you need.  E.g.

import java.io.Scanner;

Then a reader can easily see what your dependencies are.  Many IDEs will manage dependencies for you.

Naming

public class Candie{

The more common spelling is

public class Candy {

When I first read this problem I was wondering why the children needed candles.

The space between Candy { won't matter to the compiler but makes it easier for humans to read.

 static int candieCalculator(int[] numbers) {

As a general rule, objects and classes get noun names (e.g. Candy) while methods get verb names.  Consider

 static int calculate(int[] ratings) {

We already knew that this involved Candy, so we don't have to say that again in the method name (although you could).  An even better name in this case might be apportion, as that better reflects what we are doing with the candy.  We are apportioning them between students.  We do have to calculate that apportionment, but apportion seems a more specific (less generic) name.  And if we name it apportion, we can still name another method calculate.

Similarly, numbers is a pretty generic name.  In this case, we know that the numbers represent a "rating score" for each student, so let's base the name on that.  E.g. ratings, scores, or ratingScores.

Don't Repeat Yourself

        if (i==0) {
            if (numbers[i] > numbers[i+1]) {
                result++;
            }
        }
        else if (i==len-1) {
            if (numbers[i] > numbers[i-1]) {
                result++;
            }
        }
        else {

            if (numbers[i] > numbers[i+1]) {
                result++;
            }

            if (numbers[i] > numbers[i-1]) {
                result++;
            }
        }

This is longer than it needs to be.  You currently check for each edge condition and then apply a general condition if they aren't true.  Instead, consider the real problem:  if i == 0, you don't want to subtract from it as the index would be out of bounds.  So you can just say

        if (i + 1 < ratings.length) {
            if (ratings[i] > ratings[i+1]) {
                result++;
            }
        }

        if (i > 0) {
            if (ratings[i] > ratings[i-1]) {
                result++;
            }
        }

In the typical case, you'll do the same number of comparisons as previously.  The only time you do more is if i is equal to 0.  But you can make that back by getting rid of

    if (len==1) {
        return 1;
    }

As the new code will handle that case.  This will do two extra comparisons in that case, but that shouldn't matter as that case is pretty fast regardless.

Correctness

I don't believe that this returns the desired result for all inputs.  Consider

  7 1 2 3 4 5 6 7

This has an easy solution of \$1+2+3+4+5+6+7 = 28\$ but your code will instead return 13.  But that's not enough candy!  We can easily see that the first child must have a piece.  So the higher rated second child needs two pieces, etc.

Dynamic Programming

I would agree that this is not dynamic programming.  As a general rule, dynamic programming is a system to reduce recursive calculations.  This doesn't recurse at all.

You should think about how to express the problem in terms of subsets of the problem.  For example, if you know the apportionments for all the children less than i and greater than i, then what is the apportionment for i?

If you figure that out, you will have fixed the bug.

Hint:  what do you do if the apportionment for i is 0?  Obviously that's an incorrect answer to the problem.  So what should you do in that case?


Scraping a webpage copying with the logic of scrapy Today, while coming across a tutorial made by ScrapingHub on Scrapy about how it usually deals with a webpage while scraping it's content. I could see that the same logic applied in Scrapy can be applied in regular method in python.

So, I tried to make one. Here is what I did. My scraper opens a webpage, parses the 10 tags from it's right sided area then tracks down each tags and then again follows its pagination and parses the whole content. Hope I did it the right way. Here is the code:

import requests ; from lxml import html

core_link = "http://quotes.toscrape.com/"

def quotes_scraper(base_link):
    response = requests.get(base_link)
    tree = html.fromstring(response.text)
    for titles in tree.cssselect("span.tag-item a.tag"):
        processing_docs(core_link + titles.attrib['href'])

def processing_docs(base_link):
    response = requests.get(base_link).text
    root = html.fromstring(response)
    for soups in root.cssselect("div.quote"):
        quote = soups.cssselect("span.text")[0].text
        author = soups.cssselect("small.author")[0].text
        print(quote, author)

    next_page = root.cssselect("li.next a")[0].attrib['href'] if root.cssselect("li.next a") else ""
    if next_page:
        page_link = core_link + next_page
        processing_docs(page_link)

quotes_scraper(core_link)

Even though you are trying to mimic what Scrapy spider might look like, there is a very major high-level difference between how your code is executed and how a Scrapy spider is.

Scrapy is entirely asynchronous since it is based on a twisted network library which makes the code operate in a non-blocking nature, to quote the documentation:

  Requests are scheduled and processed asynchronously. This means that
  Scrapy doesn’t need to wait for a request to be finished and
  processed, it can send another request or do other things in the
  meantime. This also means that other requests can keep going even if
  some request fails or an error happens while handling it.

But, in your case, you do go through the whole process in a blocking mode, processing requests one by one:

Extract tag links from the main page
Make a request to the next tag in the list and wait for the response
Extract quotes and print them
Extract the link to the next page and go to step number 2^

Every time requests make a request, you wait. Scrapy would do things while waiting, that's the main high-level difference.

And, by the way, if you would not assign priority values for your Scrapy requests (by default all requests have the same priority), there is no enforced order in which requests are gonna be processed.


Simple download manager based on Youtube-dl There is a personal program of mine which logs in, scrape links and then download them. The downloader part is based on Youtube-dl and I think it's a smell code. Is there any way to improve this code?
import requests
import subprocess
import getpass
import time
from bs4 import BeautifulSoup as bs
import argparse

def Login(url, login_route, username, password):

    URL = url
    LOGIN_ROUTE = login_route

    headers = {
        'User-Agent': '',
        'origin': URL,
        'referer': URL + LOGIN_ROUTE,
    }

    request_session = requests.session()

    csrf_token = request_session.get(URL).cookies['csrftoken']

    login_payload = {
        'hidden_username': username,
        'password': password,
        'csrfmiddlewaretoken': csrf_token
    }

    login_request = request_session.post(
        URL + LOGIN_ROUTE, headers=headers, data=login_payload)

    # !
    msg = (
        f'You have logged in successfully {login_request.status_code}' if login_request.status_code == 200 else
        f'Error {login_request.status_code}'
    )

    print(msg)

def get_user_input():

    url = 'https://maktabkhooneh.org/signin/?next=/dashboard/'
    username = getpass.getpass('Username: ')
    password = getpass.getpass('Password: ')
    login_route = '/auth/login-authentication'

    return(Login(url, login_route, username, password))

def Scraper(page_url):

    Page_URL = page_url

    headers = {
        'User-Agent': ''
    }

    page = requests.get(
        Page_URL,
        headers=headers,
    )
    soup = bs(page.text, "html.parser")

    URL_List = []
    link_count = 0
    for a_tag in soup.select('a[href^="/course/"]'):

        links = "https://maktabkhooneh.org" + a_tag["href"]

        URL_List.append(links)
        link_count += 1

    return URL_List

def Downloader(url_list):

    URL_List = Scraper(url_list)
    download_count = 0
    for links in URL_List:
        command = f"youtube-dl {links}"
        result = subprocess.call(command, shell=True)

        # !
        if result == 0:
             download_count += 1
             return('\nReturned Value', result)

    return(f'\n{download_count} file(s) have been downloaded')

page_url = 'https://maktabkhooneh.org/course/%D8%A2%D9%85%D9%88%D8%B2%D8%B4-%D8%B1%D8%A7%DB%8C%DA%AF%D8%A7%D9%86-%DB%8C%D8%A7%D8%AF%DA%AF%DB%8C%D8%B1%DB%8C-%D9%85%D8%A7%D8%B4%DB%8C%D9%86-Andrew-NG-mk1085/%D9%81%D8%B5%D9%84-%D8%A7%D9%88%D9%84-%D9%85%D9%82%D8%AF%D9%85%D9%87-ch3364/%D9%88%DB%8C%D8%AF%DB%8C%D9%88-%D8%AE%D9%88%D8%B4%D8%A2%D9%85%D8%AF%DB%8C%D8%AF-%DB%8C%D8%A7%D8%AF%DA%AF%DB%8C%D8%B1%DB%8C-%D9%85%D8%A7%D8%B4%DB%8C%D9%86/'

Login_ = input('Login required Website [Y], [N]? ')
# !
if Login_ == 'y' or 'Y':
    get_user_input()

list_len = len(Scraper(page_url))
# !
Download_Permission = input(
    f'\n{list_len} link(s) have been extracted. Do you want to DOWNLOAD them [Y], [N]? ')
# !
if Download_Permission == 'y' or 'Y':
    Downloader(page_url)

Any help would be highly appreciated.

Issues
The first one is the use of the or operator. If you wish to compare a variable with two values, you will have to write the comparison operator twice.
For instance,
if Login_ == 'y' or 'Y':
  ...

should be write as:
if Login_ == 'y' or Login_ == 'Y':

Also, it seems you are returning from the Downloader function before iterating though all links (whenever the first process finishes correcly):
...
if result == 0:
  download_count += 1
  # This will return after one commad executes successfully
  return('\nReturned Value', result)

Code style
Now, regarding code smells and style, in python there is PEP-8, a general guide for code style.
For example, functions/methods and variable names should start with lowecase (Login should be called login).
Also, in the Login function, you are assigning the values of the url and login_route parameters to URL and LOGIN_ROUTE but they do not seem to serve for anything special. You could just use the function's parameters directly:
...
headers = {
 'User-Agent': '',
  'origin': url,
  'referer': url + login_route,
}
...
csrf_token = request_session.get(url).cookies['csrftoken']

Same goes for Page_URL in Scraper.
Moreover, return should be used as a statement, rather than with function notation:
def myFunction():
  return 0

Is better than:
def myFunction():
  return(0)

Readability
Lastly, I would say this assignment in Login is not quite readable:
msg = (
        f'You have logged in successfully {login_request.status_code}' if login_request.status_code == 200 else
        f'Error {login_request.status_code}'
    )

Language features are there for when they are useful, but readability is key and should be kept in mind. I would write that assignment as a simple if-else:
if login_request.status_code == 200:
  msg = f'You have logged in successfully {login_request.status_code}'
else:
  msg = f'Error {login_request.status_code}'


Leetcode 4sum problem using hashmaps 4sum problem

  Given an array S of n integers, are there elements a, b, c, and d in S
  such that a + b + c + d = target? Find all unique quadruplets in the
  array which gives the sum of target.

  Note: The solution set must not contain duplicate quadruplets.

Idea is to put all the pair sums a in hashmap along with corresponding indexes and once done check if -a is also present in the hashmap. If both a and -a is present and since the question is looking for unique quadruplets then we can just filter out with indexes.

class Solution(object):
    def fourSum(self, arr, target):
        seen = {}
        for i in range(len(arr)-1):
            for j in range(i+1, len(arr)):
                if arr[i]+arr[j] in seen:
                    seen[arr[i]+arr[j]].add((i,j))
                else:
                    seen[arr[i]+arr[j]] = {(i,j)}
        result = []
        for key in seen:
            if -key + target in seen:
                for (i,j) in seen[key]:
                    for (p,q) in seen[-key + target]:
                        sorted_index = sorted([arr[i], arr[j], arr[p], arr[q]])
                        if i not in (p, q) and j not in (p, q) and sorted_index not in result:
                            result.append(sorted_index)
        return result

Use enumerate rather than range(len(...)) + __getitem__. It is both faster and more readable.
To limit items of the second iteration to be "after the current item" you can use itertools.combinations.
To avoid the need to check for the special case of "is the item already in the dictionary?", use a collections.defaultdict.
You could use a set rather than a list to store the final results and remove yourself the need to check for duplicates
-key + target is better written as target - key

import itertools
from collections import defaultdict

def four_sum(array, target):
    seen = defaultdict(set)
    for (i, first), (j, second) in itertools.combinations(enumerate(array), 2):
        seen[first + second].add((i, j))

    result = set()
    for key, first_indices in seen.items():
        second_indices = seen.get(target - key, set())
        for p, q in second_indices:
            for i, j in first_indices:
                # Not reusing the same number twice
                if not ({i, j} & {p, q}):
                    indices = tuple(sorted(array[x] for x in (i, j, p, q)))
                    result.add(indices)
    return result


Styling the font size in a nav bar The font size scales up/down based on the size of the browser to fit the nav bar buttons (using @media).

.navButtonText p{
    text-align: center;
    margin: auto;
    font-family: 'Indie Flower', serif;
                 font-weight:900;
                 color: gray;
                 font-size: 20px;
line-height: 20px;
  vertical-align: middle;
    margin-top: 6px;
}
}
@media only screen and (max-width: 650px) {
    .navButtonText p{
    text-align: center;
    margin: auto;
    font-family: 'Indie Flower', serif;
                 font-weight:900;
                 color: gray;
                 font-size: 18px;
line-height: 20px;
  vertical-align: middle;
    margin-top: 6px;
}
}
@media only screen and (max-width: 550px) {
    .navButtonText p{
    text-align: center;
    margin: auto;
    font-family: 'Indie Flower', serif;
                 font-weight:900;
                 color: gray;
                 font-size: 16px;
line-height: 20px;
  vertical-align: middle;
    margin-top: 6px;
}
}
@media only screen and (max-width: 450px) {
    .navButtonText p{
    text-align: center;
    margin: auto;
    font-family: 'Indie Flower', serif;
                 font-weight:900;
                 color: gray;
                 font-size: 14px;
line-height: 20px;
  vertical-align: middle;
    margin-top: 6px;
}
}
@media only screen and (max-width: 350px) {
    .navButtonText p{
    text-align: center;
    margin: auto;
    font-family: 'Indie Flower', serif;
                 font-weight:900;
                 color: gray;
                 font-size: 12px;
line-height: 20px;
  vertical-align: middle;
    margin-top: 6px;
}
}

As you can see, it is quite a bit of styling for one thing. I was wondering if there was a way I could clean up this code, so I wouldn't need to have it in these many lines.

I have SASS and Compass which I am willing to use.

You could remove all the duplicate lines in the @media queries and provide only the changes in these tags. This is beacause in CSS, the CSS classes that are interpreted by the browser down-most on your page, are the ones prevailing in your final design. All previous defined CSS rules apply unless overwritten. In your case this means that only the font-size changes and should be redefined:

.navButtonText p {
    text-align: center;
    margin: auto;
    font-family: 'Indie Flower', serif;
    font-weight: 900;
    color: gray;
    font-size: 20px;
    line-height: 20px;
    vertical-align: middle;
    margin-top: 6px;
}
@media only screen and (max-width: 650px) {
    .navButtonText p {
        font-size: 18px;
    }
}
@media only screen and (max-width: 550px) {
    .navButtonText p {
        font-size: 16px;
    }
}
@media only screen and (max-width: 450px) {
    .navButtonText p {
        font-size: 14px;
    }
}
@media only screen and (max-width: 350px) {
    .navButtonText p {
        font-size: 12px;
    }
}

FYI: there is away to prevent this from happening, using the !important tag.


Simple code that checks if you're old enough to drive I made this small system that checks if you're old enough to drive. I'm new to C#, but I do know most of the basics since I also know Java. I'm trying to make my code more efficient. Any suggestions or help would be appreciated.

using System;
using System.Threading;

class EfficientCode
{

  static int age = 0;

    static void Main(string[] args)
    {
        Console.WriteLine("This is just a simple project that takes simple and small information from you and uses it to process the information and checks whether you're allowed to drive or not. It also tells you how long is left till you drive.");

        Thread.Sleep(8000); // adds a 8 second delay so that the user could first read the introduction

        Start(); // after the 8 second delay it starts the system
    }

    static void Start()
    {
        Console.WriteLine("Enter your first name");
        String firstName = Console.ReadLine();

        Console.WriteLine("Enter your middle name");
        String middleName = Console.ReadLine();

        Console.WriteLine("Enter your last name");
        String lastName = Console.ReadLine();

        Console.WriteLine("Enter your age");

        try
        {
            age = Convert.ToInt32(Console.ReadLine());
        }
        catch
        {
            Console.WriteLine("ERROR: Input AGE must be a number!");
            Console.WriteLine("Reinitializing questions.");

            Thread.Sleep(3000); // this adds a delay of 3 seconds

            Start(); // restarts the questions
        }

        if (ConfirmInformation(firstName, middleName, lastName, age))
        {
            if (age >= 18)
            {
                Console.WriteLine("Congratulations! You're already old enough to drive.");

                Environment.Exit(0); // Ends the system
            }
            else if (age == 17)
            {
                Console.WriteLine("You're almost there! You can get a learners permit.\nYou have 1 more year left to get an official drivers license!");

                Environment.Exit(0); // Ends the system
            }
            else
            {
                Console.WriteLine("You'll get there. You're not old enough to have a license.");

                int yearsLeft = 18 - age; // gets the years left to drive

                Console.WriteLine("You still have " + yearsLeft + " years left to get a license.");

                Environment.Exit(0); // Ends the system
            }
        }
        else
        {
            Console.WriteLine("Reinitializing questions.");

            Thread.Sleep(3000); // this adds a delay of 3 seconds

            Start(); // restarts the questions
        }
    }

    static bool ConfirmInformation(String firstName, String middleName, String lastName, int age)
    {
        Console.WriteLine("======================================");
        Console.WriteLine("INFORMATION CHECK:");
        Console.WriteLine("");
        Console.WriteLine("FIRST NAME: " + firstName);
        Console.WriteLine("MIDDLE NAME: " + middleName);
        Console.WriteLine("LAST NAME: " + lastName);
        Console.WriteLine("AGE: " + age);
        Console.WriteLine("");
        Console.WriteLine("Are all those information correct? (Y/N)");
        Console.WriteLine("======================================");

        String booleanInput = Console.ReadLine();

        if (booleanInput.Equals("yes", StringComparison.OrdinalIgnoreCase) || booleanInput.Equals("y", StringComparison.OrdinalIgnoreCase))
        {
            return true;
        }
        else if (booleanInput.Equals("no", StringComparison.OrdinalIgnoreCase) || booleanInput.Equals("n", StringComparison.OrdinalIgnoreCase))
        {
            return false;
        }
        else
        {
            Console.WriteLine("ERROR: Input must be either YES or NO.");

            return false;
        }

        return false;
    }

}

class EfficientCode could be improved by naming the class by the purpose of the code. Otherwise your naming is good.
static int age = 0; should be a local variable in Start()
Thread.Sleep(8000); // adds a 8 second delay so that the user could first read the introduction shouldn't be needed because you aren't clearing the Console hence the user could still read it.
Instead of try..catch you could take advantage of the int.TryParse() method which returns a bool indicating the success of "converting" the string to an int. If you would use int.TryParse like below you could omit the restarting in case of an error. If the call succeeds age holds the integer.

while(!int.TryParse(Console.ReadLine(), out age))
{
    Console.WriteLine("ERROR: The entered AGE must be a number!");
    Console.WriteLine("Enter your age");
}

ConfirmInformation() is doing a little bit to much. You should split it into 2 methods. One composing the output and one printing the output and asking for confirmation.

private static string ComposeOutput(String firstName, String middleName, String lastName, int age)
{
    StringBuilder sb = new StringBuilder(100);
    sb.AppendLine("======================================")
      .AppendLine("INFORMATION CHECK:")
      .AppendLine()
      .AppendLine($"FIRST NAME: {firstName}")
      .AppendLine($"MIDDLE NAME: {middleName}")
      .AppendLine($"LAST NAME: {lastName}")
      .AppendLine($"AGE: {age}")
      .AppendLine()
      .AppendLine("Are all those information correct? (Y/N)")
      .AppendLine("======================================");

    return sb.ToString();
}

This method uses the StringBuilder class. The AppendLine() method is returning the StringBuilder itself hence we can use the method calls fluently. It uses $-string interpolation as well.

The former ConfirmInformation() method could then look like so

private static bool ConfirmInformation(string information)
{
    Console.Write(output);

    while (true)
    {
        string input = Console.ReadLine().ToLowerInvariant();
        if (input == "y" || input == "yes")
        {
            return true;
        }
        if (input == "n" || input == "no")
        {
            return false;
        }

        Console.WriteLine("ERROR: Input must be either YES or NO.");
        Console.WriteLine();
        Console.Write(output);
    }
}

I have made a while(true) loop here because your user wouldn't want to restart the whole process if they made a mistake.
As a sidenode, the variable String booleanInput = Console.ReadLine(); is misleading. If Sam the maintainer would read this method the purpose of that variable wouldn't be seen at first glance.


Python subclassing ConfigParser I've never done anything like this before so I'd like someone else to look at this before I get too carried away :)

Am I making this more complicated than it needs to be?  I'm trying to make it EASY for other modules/scripts on this system to store and retrieve their settings. Hence why I trap the ConfigParser.NoOptionError error and return None and create the section if it doesn't exist in the set() method.

Suggestions?

import ConfigParser
import os

from ast import literal_eval as Eval

class _ConfParse(ConfigParser.ConfigParser):
    def __init__(self, confpath, conffile):
        ConfigParser.ConfigParser.__init__(self)
        self.conf_file = os.path.join(confpath, conffile)
        try: self.readfp(open(self.conf_file), 'r')
        except IOError as Err:
            if Err.errno == 2: pass
            else: raise Err

    def set(self, section, option, value):
        if self.has_section(section):
            ConfigParser.ConfigParser.set(self, section, option, str(value))
        else:
            self.add_section(section)
            ConfigParser.ConfigParser.set(self, section, option, str(value))

    def get(self, section, option):
        try: return Eval(ConfigParser.ConfigParser.get(self, section, option))
        except ConfigParser.NoOptionError: return None

    def save(self):
        self.write(open(self.conf_file, 'w'))

    def __del__(self):
        self.save()

class LocalConfig(_ConfParse):
    def __init__(self, conffile, confpath = '/etc/local/cnf'):
        _ConfParse.__init__(self, confpath, conffile)

class SysConfig(_ConfParse):
    def __init__(self, conffile, confpath = '/etc/sys/cnf'):
        _ConfParse.__init__(self, confpath, conffile)

One problem I see is that returning None from your modified get() method conflicts with the normal case of a valueless option (from the bottom of the module docs for ConfigParser):

>>> import ConfigParser
>>> import io

>>> sample_config = """
... [mysqld]
... user = mysql
... pid-file = /var/run/mysqld/mysqld.pid
... skip-external-locking
... old_passwords = 1
... skip-bdb
... skip-innodb
... """
>>> config = ConfigParser.RawConfigParser(allow_no_value=True)
>>> config.readfp(io.BytesIO(sample_config))

>>> # Settings with values are treated as before:
>>> config.get("mysqld", "user")
'mysql'

>>> # Settings without values provide None:
>>> config.get("mysqld", "skip-bdb")

>>> # Settings which aren't specified still raise an error:
>>> config.get("mysqld", "does-not-exist")
Traceback (most recent call last):
  ...
ConfigParser.NoOptionError: No option 'does-not-exist' in section: 'mysqld'

Note the second-to-last example commented as "Settings without values provide None:"  Of course this isn't an issue if you intend to exclude this sort of option.  Other than that, I like the auto-section feature.

Though, I'd lean towards adding to the interface rather than masking and changing the behavior, so instead of replacing get/set, add safe_ versions:

def safe_set(self, section, option, value):
    if self.has_section(section):
        self.set(section, option, str(value))
    else:
        self.add_section(section)
        self.set(section, option, str(value))

def safe_get(self, section, option):
    if self.has_option(section, option):
        return self.get(section, option)
    else:
        return None

This would make it more flexible as code would still have access to ConfigParser's normal interface and the option of using the "safe" calls which don't throw exceptions.


Review of HTML markup for this blog concept I am creating a blog concept with this layout on CodePen

Here is my current code in development

Skating Down Venice Beach

Venice Beach

Literally trust fund Helvetica dreamcatcher selfies. Pinterest aesthetic organic Echo Park artisan meggings tousled Tumblr, Pitchfork gentrify raw denim yr you probably haven't heard of them banjo. Street art Wes Anderson ethnic ethical authentic, High Life swag ennui. Wolf cardigan fingerstache gentrify, PBR&B cray XOXO vegan deep v tote bag ethnic. Banh mi you probably haven't heard of them seitan meh Austin iPhone. High Life wolf Tonx, dreamcatcher lo-fi seitan ethnic pop-up fingerstache whatever. Trust fund Portland ethnic church-key, Tumblr squid hoodie dreamcatcher +1 seitan.

When I move to production, I will use a MustacheJS template in Meteor. So was planning on this

<template name="posts">

{{title}}

{{city}}

      {{{content}}}

</template>

Is there any room for improvement here? for example, will I be better off using
s? or should I perhaps change anything else?

Your img is missing the alt attribute.

I assume that here a graphic will be inserted via CSS? Then this will not be accessible by users without CSS support, and it will probably not be accessible by screen reader and/or keyboard users. The link should have content, i.e., an img (with alt attribute) or text ("Read more") (which can be visually hidden, if required).

Also, you shouldn’t use the i element in such a case (as your use doesn’t match i’s definition). Go with span if you need an element.

Why did you skip a heading level (from h2 to h4)? Either use h3 for the second heading, or go with h1 everywhere (but then you need to use sectioning elements explicitly!):

Skating Down Venice Beach

Venice Beach

If the text titled "Venice Beach" is just some general information about it (not unique to this image), consider to use the aside element instead of section.


Splitting an Excel file into separate sheets by data I have a VBA macro for a file of GPS data. It copies rows and subsequent rows to a new worksheet when the GPS data shows movement to the south.

It works fine, but is pretty slow on larger files (20,000 rows+). I'm looking for suggestions on how to speed it up.

The cell values in the macro are from another macro analyzing the GPS data. 1 means movement North, 2 means no movement, 3 means movement South. I could easily change these to different datatype if that would speed up this macro.

Sub SplitIntoGroups()

Application.ScreenUpdating = False

'The data is incomplete in the first 50 and last 50 rows, so this is so it doesn't analyze that data

Dim NearEnd As Integer
Dim NearStart As Integer
NearStart = 52
NearEnd = (Cells(Sheet1.Rows.Count, 3).End(xlUp).row) - 50

'Groups are the different sheets of data. I start with zero, because the first is usually unimportant

Dim Groups As Integer
Dim r As Integer

'GroupsCheck is the variable used to go through each row
Dim GroupsCheck As Integer

'WentNorth is a boolean to ensure there was movement to the north before creating a new sheet.
Dim WentNorth As Boolean

Sheets.Add.Name = "Group 0"
Groups = 0
'r is used as the row offset, so each row can be copied on the next row in the proper worksheet
r = 0

For GroupsCheck = NearStart To NearEnd

    If Sheets("Sheet1").Cells(GroupsCheck, "AB").Value = 1 Then WentNorth = True
    End if
    r = r + 1

    'once north movement (WentNorth=true) exists, a new sheet will be created when south movement first occurs (cells = 3 and previous cell <3)
        If Sheets("Sheet1").Cells(GroupsCheck, "AB").Value = 3 And Sheets("Sheet1").Cells(GroupsCheck - 1, "AB").Value < 3 And WentNorth = True Then
                Groups = Groups + 1
                Sheets.Add.Name = "Group " & Groups

                'once a new sheet is created, r and WentNorth are reset
                r = 0
                WentNorth = False
        End If

    'the copying function
    Worksheets("Sheet1").Range("A" & GroupsCheck & ": AB" & GroupsCheck).Copy Worksheets("Group " & Groups).Range("A2:AE2").Offset(r, 0)
Next GroupsCheck

Application.ScreenUpdating = True

End Sub

20,000+ is pretty close to 32,767, so first thing is to declare NearEnd As Long. Actually, you can declare all variables used as row numbers as Long, there is no gain on performance when you use Integer, and you are less prone to have overflow error.
Secondly, the issue with Worksheets("Group " & Groups).Range("A2:AE2").Offset(r, 0) line. As mentioned in comments above, it should be either continuation sing in line above "(space)_" or .pastespecial on the end.
And third, you could make code running faster just by replacing For-Next loop with the Object Loop. So, define range:

With Sheets("Sheet1")
    Set rng = Range(.Range(NearStart, "AB"), .Range(NearEnd, "AB"))
End With

and then

For Each cl in rng
(....)
Next cl

rng and cl declared as Range, of course.

If you have formulas and they don't need to be calculated while macro is running, I should turn of automatic calculation, together with screen flickering. You probably know this, just for the record:

With Application
    .ScreenUpdating = False
    .Calculation = xlCalculationmanual
End With

and back to xlCalculationAutomatic on the end.


RSS to JSON for insertion into MongoDB In my code, there is a chain of functions call where the deepest function may throw an exception. I catch and handle it according to the business logic. The problem is that all other functions in the call chain are dependent on this function, thus, if the deepest function returns null all others function will throw the NullPointerException.

My question is, from the code style/maintainability/best practices point of view, can I care about the exception only at the deepest call chain level and in all higher function in the chain use something like catch (NullPointerException ignored) {}?

Is it a good idea at all to use ignored in a try-catch expression or should I really address to all catches even if they are just the consequences of the execution of another function, where I do the proper exception handling?

The code fragment:

public static JSONObject jsonBuilder(String xmlDataSource) {

    JSONObject xmlJSONObj = null;

    try {
        xmlJSONObj = XML.toJSONObject(xmlDataSource);
    } catch (JSONException e) {
        e.printStackTrace();
    }

    return xmlJSONObj;
}

public static void fetchPushData(DBDriver drv, MongoCollection<Document> dbColl, TrackedEpisode episode) {

    String xmlFeed = JsonHandler.xmlDataLoader(episode.getUrlRSS());

    try {
        JSONObject xmlJSONObj = JsonHandler.jsonBuilder(xmlFeed);

        JsonHandler.docBuilderEvent(xmlJSONObj, episode.getEpisodeID());

        JSONArray itemsArr = JsonHandler.getItemsArr(xmlJSONObj);

        episode.setEpisodeLastUpdate(getLatestPubDate(itemsArr));

        drv.insertToDB(dbColl, itemsArr);

    } catch (NullPointerException ignored) {}
}

Here we can see that in case of failure jsonBuilder() throws exception, which I handle and returns null. Now, the docBuilderEvent() will also throw NullPointerException due to null in xmlJSONObj. Should I care about this NullPointerException or it's enough to handle it only in the jsonBuilder()?

No, that's not a good design. Catching NullPointerException is almost never the correct thing to do. Effectively ignoring it is worse. You're hiding possible programming errors by using it in this way.

jsonBuilder() should be throwing a known, checked exception up to fetchPushDriver(), which should either handle the exception, rethrow it, or wrap it in a new exception and throw that.