Website Navigation Bar - Potential code soup I've had a bit of help designing the header for a website here: http://jsfiddle.net/gxLt5/21/

It looks and works exactly how I had hoped. That said, I do feel like the code is a bit grotty, and I'm sure there is a more elegant solution to achiving the same result.

Could anyone please give me some feedback on the HTML and CSS so see if it can be improved or restructured?

HTML

A

B

 Test Test

Log out

CSS

/* 1.0 Resets
************************************/
 * {
    margin: 0;
    padding: 0;
}
ul {
    font-size: 0;
}
html, body {
    height: 100%;
    max-height: 100%;
}
body {
    font-family:"Helvetica Neue", Helvetica, Arial, sans-serif;
    font-size: 13px;
    background-color: white;
}
li {
    list-style: none;
}
a {
    text-decoration: none;
}
/* 2.0 Layout
************************************/
 .header {
    background-color: #9B59B6;
    height: 50px;
}
.logo {
    height: 50px;
    margin-bottom: 30px;
}
.navigation {
    position: absolute;
    display:inline-block;
    z-index: 1000;
        width: 100%;
}
.navigation li:not(:last-child){
   float:left
}
.navigation li a {
    display: block;
    height: 50px;
    line-height: 50px;
    color: #fff;
    font-family:'Helvetica Neue', Helvetica, Arial, sans-serif;
    font-size: 14px;
    padding-left: 30px;
    padding-right: 30px;
    opacity: 0.7;
}
.navigation li a:hover, .navigation li.active a {
    opacity: 1;
}
.navigation li span {
    display: inline-block;
    background: url(http://i.imgur.com/R6Fjt4e.png) no-repeat left center;
    width: 16px;
    height: 16px;
    /*overflow:hidden;*/
    margin-right: 16px;
}
.container {
    padding-top: 14px;
}
.dialog {
    display: none;
}
/* 4.0 Navigation Styles
************************************/
 #dashboard span {
    background-position: -64px -0px;
}
#vacancies span {
    background-position: -160px 0;
}
#candidates span {
    background-position: -32px -0;
}
#company span {
    background-position: -288px 0;
}
#account span {
    background-position: -128px 0;
}

.right {
    float: right;
}
nav ul ul {
    display: none;
}
nav ul li:hover > ul {
    display: block;
    position: absolute;
    background-color: #9B59B6;
}
.dropdown li {
    display: block;
}

First, I'm not sold on your use of  as a header. You can assign an aria role=banner to it or replace it with the tag intended to be a . I understand you're using your logo as a navigation link (e.g. home), however, you still have to make the browser understand what your header is.

Working off your code:

judging from the css used in your code I'm assuming IE8 support isn't required

If you're going to use a  for an icon (I'm assuming your trying to avoid using a content image), I'd recommend using an svg instead, see Fiddle (this uses natural elements to specify header/nav). There are plenty of SVG Fallbacks for browsers that don't support SVG.

If you want to forgo using a "content" image I'd use pseudo elements :before or :after to place the icon, see Fiddle. (this uses aria roles to specify header/nav)

How I'd approach it:

Your setup is, for the most part, solid. However, I'd go about it a little differently. Check this Fiddle.
(disclaimer: I wrote this up while still intoxicated from the fourth, so apologize ahead of time as it could probably be written better)

HTML

Title

A

B

         Test

Logout

CSS

* {
    margin:0;
    padding:0}
*,
*:before,
*:after{
    -webkit-box-sizing: border-box;
    -moz-box-sizing: border-box;
    box-sizing: border-box }

html,
body{
    height: 100%;
    max-height: 100%}
html{
    -ms-text-size-adjust: 100%;
    -webkit-font-smoothing: antialiased;
    -webkit-text-size-adjust: 100%;
    font-family: sans-serif;
    text-shadow: 1px 1px 1px rgba(0,0,0,.004)}
body{
    font-family:"Helvetica Neue", Helvetica, Arial, sans-serif;
    font-size:13px;
    font-size:.8125rem;
    background-color:white}
a{text-decoration:none}
.visuallyhidden{
  border:0;
  clip:rect(0,0,0,0);clip:rect(0000);
  height:1px;height:.0625rem;
  margin:-1px;margin:-.0625rem;
  overflow:hidden;
  padding:0;
  position:absolute;
  width:1px;width:.0625rem}
.header{
    background-color:#9B59B6;
    padding:0 1.5625rem}
.navigation{
    display:inline-block;
    padding-left:25px;
    width:calc(100% - 80px)}
.navbar{list-style:none}
.navbar li{
    display:inline-block;
    float:left;
    position:relative}
.navbar li a:before{
    content:'';
    width:16px; width:1rem;
    height:16px; height:1rem;
    position:absolute;
    top:50%;
    left:10px;
    margin-top:-8px;
    background:url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4KPCEtLSBHZW5lcmF0ZWQgYnkgSWNvTW9vbi5pbyAtLT4KPCFET0NUWVBFIHN2ZyBQVUJMSUMgIi0vL1czQy8vRFREIFNWRyAxLjEvL0VOIiAiaHR0cDovL3d3dy53My5vcmcvR3JhcGhpY3MvU1ZHLzEuMS9EVEQvc3ZnMTEuZHRkIj4KPHN2ZyB2ZXJzaW9uPSIxLjEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHdpZHRoPSIxNiIgaGVpZ2h0PSIxNiIgdmlld0JveD0iMCAwIDE2IDE2Ij4KPGc+CjwvZz4KCTxwYXRoIGQ9Ik05LjczMiAxMC45OGMtMC4zNDUtMC4wNTUtMC4zNTMtMS4wMDUtMC4zNTMtMS4wMDVzMS4wMTUtMS4wMDUgMS4yMzYtMi4zNTZjMC41OTUgMCAwLjk2My0xLjQzNyAwLjM2OC0xLjk0MiAwLjAyNS0wLjUzMiAwLjc2NS00LjE3Ny0yLjk4Mi00LjE3N3MtMy4wMDcgMy42NDUtMi45ODIgNC4xNzdjLTAuNTk1IDAuNTA1LTAuMjI4IDEuOTQyIDAuMzY4IDEuOTQyIDAuMjIxIDEuMzUxIDEuMjM2IDIuMzU2IDEuMjM2IDIuMzU2cy0wLjAwOCAwLjk1LTAuMzUzIDEuMDA1Yy0xLjExMyAwLjE3Ny01LjI2OCAyLjAxMC01LjI2OCA0LjAyMGgxNGMwLTIuMDEwLTQuMTU1LTMuODQzLTUuMjY4LTQuMDIweiIgZmlsbD0iI2ZmZmZmZiI+PC9wYXRoPgo8L3N2Zz4K) no-repeat}
.navbar li:last-of-type{float:right}
.navbar li:last-of-type .dropdown:before{background:url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4KPCEtLSBHZW5lcmF0ZWQgYnkgSWNvTW9vbi5pbyAtLT4KPCFET0NUWVBFIHN2ZyBQVUJMSUMgIi0vL1czQy8vRFREIFNWRyAxLjEvL0VOIiAiaHR0cDovL3d3dy53My5vcmcvR3JhcGhpY3MvU1ZHLzEuMS9EVEQvc3ZnMTEuZHRkIj4KPHN2ZyB2ZXJzaW9uPSIxLjEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHdpZHRoPSIxNiIgaGVpZ2h0PSIxNiIgdmlld0JveD0iMCAwIDE2IDE2Ij4KPGc+CjwvZz4KCTxwYXRoIGQ9Ik0xNC41OSA5LjUzNWMtMC44MzktMS40NTQtMC4zMzUtMy4zMTcgMS4xMjctNC4xNjRsLTEuNTcyLTIuNzIzYy0wLjQ0OSAwLjI2My0wLjk3MiAwLjQxNC0xLjUyOSAwLjQxNC0xLjY4IDAtMy4wNDItMS4zNzEtMy4wNDItMy4wNjJoLTMuMTQ1YzAuMDA0IDAuNTIyLTAuMTI2IDEuMDUxLTAuNDA2IDEuNTM1LTAuODM5IDEuNDU0LTIuNzA2IDEuOTQ4LTQuMTcgMS4xMDZsLTEuNTcyIDIuNzIzYzAuNDUzIDAuMjU3IDAuODQ1IDAuNjM0IDEuMTIzIDEuMTE3IDAuODM4IDEuNDUyIDAuMzM2IDMuMzExLTEuMTIgNC4xNmwxLjU3MiAyLjcyM2MwLjQ0OC0wLjI2MSAwLjk2Ny0wLjQxIDEuNTIyLTAuNDEgMS42NzUgMCAzLjAzMyAxLjM2MiAzLjA0MiAzLjA0NmgzLjE0NWMtMC4wMDEtMC41MTcgMC4xMjktMS4wNDAgMC40MDYtMS41MTkgMC44MzgtMS40NTIgMi43LTEuOTQ3IDQuMTYzLTEuMTFsMS41NzItMi43MjNjLTAuNDUtMC4yNTctMC44MzktMC42MzMtMS4xMTYtMS4xMTN6TTggMTEuMjRjLTEuNzg5IDAtMy4yNC0xLjQ1LTMuMjQtMy4yNHMxLjQ1LTMuMjQgMy4yNC0zLjI0YzEuNzg5IDAgMy4yNCAxLjQ1IDMuMjQgMy4yNHMtMS40NSAzLjI0LTMuMjQgMy4yNHoiIGZpbGw9IiNmZmZmZmYiPjwvcGF0aD4KPC9zdmc+Cg==) no-repeat}
.navbar li a{
    display:block;
    line-height:50px;
    padding-left:37px;
    padding-right:25px;
    vertical-align: middle;
    color:white}
.navbar:after,
.navbar:before{
  content:'';
  display:table}
.navbar:after {clear:both}

.submenu{display:none}
.navbar li:last-of-type:hover>.submenu{
    display:block;
    position:absolute;
    left:0;
    background-color:#9B59B6}
.submenu li{display:block}
.submenu li a:before{background:url(data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4KPCEtLSBHZW5lcmF0ZWQgYnkgSWNvTW9vbi5pbyAtLT4KPCFET0NUWVBFIHN2ZyBQVUJMSUMgIi0vL1czQy8vRFREIFNWRyAxLjEvL0VOIiAiaHR0cDovL3d3dy53My5vcmcvR3JhcGhpY3MvU1ZHLzEuMS9EVEQvc3ZnMTEuZHRkIj4KPHN2ZyB2ZXJzaW9uPSIxLjEiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyIgeG1sbnM6eGxpbms9Imh0dHA6Ly93d3cudzMub3JnLzE5OTkveGxpbmsiIHdpZHRoPSIxNiIgaGVpZ2h0PSIxNiIgdmlld0JveD0iMCAwIDE2IDE2Ij4KPGc+CjwvZz4KCTxwYXRoIGQ9Ik0xMiAxMHYtMmgtNXYtMmg1di0ybDMgM3pNMTEgOXY0aC01djNsLTYtM3YtMTNoMTF2NWgtMXYtNGgtOGw0IDJ2OWg0di0zeiIgZmlsbD0iI2ZmZmZmZiI+PC9wYXRoPgo8L3N2Zz4K) no-repeat}

edit/note
This won't work well for mobile, so I suggest you work media queries into the nav and use jQuery or equivalent to handle the drop-down functionality.


A timer to track time spent practicing Python I made this to record the time I spend practicing Python. I'll eventually add in functionality to visualize the data. I'd appreciate any feedback that will help me improve.

# timer.py - A timer to track time spent practicing python

from datetime import timedelta
from datetime import datetime as Datetime
# used json b/c I wanted to practice it
import json
import os

# dict with all python practice options
options = ["Code Review",
           "Project",
           "Puzzles (Project Euler, Finxter, kaggle)",
           "Courses, Books, or Videos",
           "Read Docs"]

# Prompts user what type of practice they would like to do. User enters int.
def practice_choice():
    print("Pick a form of practice:\n")

    # print each option in options then ask user to select
    num = 1
    for option in options:
        print(str(num) + ". " + option)
        num += 1
    selection = input("\nPick a number: ")

    return options[int(selection) - 1]

def timer(selection):
    # store notes about practice session
    notes = input("Enter notes... ")

    # start timing session and get start time
    input("Hit Enter to begin practice...")
    start_dt = Datetime.now()

    # loop to allow for pausing. infinite loop that breaks when user clicks enter to stop session.
    var = 1
    total_so_far = 0
    while var == 1:
        pause_or_stop = input("Hit Enter to stop or space to pause...")

        # when user hits enters space char
        if pause_or_stop == ' ':

            # gets time session was paused. Calculates time practicing so far. Stores for later
            # Allows user to pause multiple times
            pause_dt = Datetime.now()
            total_so_far += int((start_dt - pause_dt) / timedelta(seconds=1))  # used sec for easier testing.

            # resume on enter key and overwrites old start time
            input("Hit Enter to resume")
            start_dt = Datetime.now()

        # breaks if enter key
        elif pause_or_stop == '':
            break

        # catch bad input
        else:
            print("Key not recognized")

    # get stop time and find total time in minutes
    end_dt = Datetime.now()
    total_mins = int((end_dt - start_dt) / timedelta(seconds=1)) + total_so_far  # used sec for easier testing.

    # add value to data[selection] dict. value is a list of datetime info, noes, total_minutes
    data[selection].append({'day': Datetime.now().strftime("%A"),  # %b %d %Y %H:%M),
                            'date': Datetime.now().strftime("%m-%d-%y"),
                            'time': Datetime.now().strftime("%H:%M"),
                            'notes': notes,
                            'minutes': total_mins})

    # convert dict to JSON object
    with open(target_file, 'w') as json_file:
        json.dump(data, json_file, indent=4)

# prompt user for type of practice
choice = practice_choice()

# practice_log file with all practice sessions are stored
target_file = "practice_log.json"

# initialize data dict which will hold practice logs
data = {}

# creates json file if it doesn't exist
if not os.path.exists(target_file):
    # top level values of json are created corresponding to practice type
    for each_item in options:
        data[each_item] = []
    with open(target_file, 'w') as json_file:
        json.dump(data, json_file, indent=4)

# open json file and load into target_files
with open(target_file, 'r') as json_file:
    data = json.load(json_file)

# starts timer
timer(choice)

Ways to improve and redesign:

I'll start with listing inner issues (duplication, redundant or over-complicated conditional/algorithms etc). Then I'll provide you with a new Object-Oriented approach as a more manageable, flexible and reliable.

options = [...]. "Practice options" is better designed as a constant with immutable items (tuple):

OPTIONS = ("Code Review",
        "Project",
        "Puzzles (Project Euler, Finxter, kaggle)",
        "Courses, Books, or Videos",
        "Read Docs")

practice_choice function:
Dragging num variable though the for loop is redundant and easily replaced with enumerate feature (with custom start= parameter):

print('\n'.join(f'{i}. {option}' for i, option in enumerate(self.OPTIONS, 1)))

timer function:
var variable here:

var = 1
total_so_far = 0
while var == 1:
    ...

is redundant and it's not reassigned anywhere. Simply go with:

while True:
    ...

total_so_far variable is better named as total_time
Datetime.now() is duplicated on constructing a dictionary of session info (notes, total minutes etc). Instead, use end_dt from the nearest statement end_dt = Datetime.now() as it's already contains the current datetime.
creating non-existent target file:
filling data dict with:

for each_item in options:
    data[each_item] = []

is easily substituted with dict comprehension:

data = {o: [] for o in self.OPTIONS}

saving data

with open(target_file, 'w') as json_file:
    json.dump(data, json_file, indent=4)

is extracted into a separate function/method with respective name

Now, the Object-Oriented PracticeTimer class with lot of benefits like:

class constants
encapsulated state/behavior
dealing with target file: initializing/saving
methods with specific responsibilities
easy to extend

Underlying Class diagram:

# timer.py - A timer to track time spent practicing python

from datetime import timedelta
from datetime import datetime as Datetime
import json
import os

class PracticeTimer:
    # practice options
    OPTIONS = ("Code Review",
               "Project",
               "Puzzles (Project Euler, Finxter, kaggle)",
               "Courses, Books, or Videos",
               "Read Docs")

    # practice_log file with all practice sessions are stored
    TARGET_FILE = "practice_log.json"

    def __init__(self):
        # creates json file if it doesn't exist
        if not os.path.exists(self.TARGET_FILE):
            # top level values of json are created corresponding to practice type
            data = {o: [] for o in self.OPTIONS}
            with open(self.TARGET_FILE, 'w') as json_file:
                json.dump(data, json_file, indent=4)

        # open json file and load session data
        with open(self.TARGET_FILE, 'r') as json_file:
            self._data = json.load(json_file)

        self._current_choice = None

    def practice_choice(self):
        """Prompts user to select practice item. User enters int."""
        print("Pick a form of practice:\n")

        # print options, then ask user to select
        print('\n'.join(f'{i}. {option}' for i, option in enumerate(self.OPTIONS, 1)))
        choice = input("\nPick a number: ")
        self._current_choice = self.OPTIONS[int(choice) - 1]
        return self._current_choice

    def _save_log(self):
        with open(self.TARGET_FILE, 'w') as json_file:
            json.dump(self._data, json_file, indent=4)

    def run(self):
        self.practice_choice()

        # store notes about practice session
        notes = input("Enter notes... ")

        # start timing session and get start time
        input("Hit Enter to begin practice...")
        start_dt = Datetime.now()

        # loop to allow for pausing. Infinite loop that breaks when user clicks enter to stop session.
        total_time = 0
        while True:
            pause_or_stop = input("Hit Enter to stop or space to pause...")

            # when user hits enters space char
            if pause_or_stop == ' ':

                # gets time session was paused. Calculates time practicing so far. Stores for later
                # Allows user to pause multiple times
                pause_dt = Datetime.now()
                total_time += int((start_dt - pause_dt) / timedelta(seconds=1))  # used sec for easier testing.

                # resume on enter key and overwrites old start time
                input("Hit Enter to resume")
                start_dt = Datetime.now()

            # breaks if enter key
            elif pause_or_stop == '':
                break
            # catch bad input
            else:
                print("Key not recognized")

        # get stop time and find total time in minutes
        end_dt = Datetime.now()
        total_mins = int((end_dt - start_dt) / timedelta(seconds=1)) + total_time  # used sec for easier testing.

        # add info to the current session (as a list of datetime info, notes, total_minutes)
        self._data[self._current_choice].append({'day': end_dt.strftime("%A"),  # %b %d %Y %H:%M),
                                                 'date': end_dt.strftime("%m-%d-%y"),
                                                 'time': end_dt.strftime("%H:%M"),
                                                 'notes': notes,
                                                 'minutes': total_mins})

        self._save_log()

if __name__ == '__main__':
    p_timer = PracticeTimer()
    p_timer.run()


Basic commenting structure for commenting on objects I would dearly love some feedback on the model below.  It's a basic commenting structure where users can comment on a variety of objects in the model.  I have denormalised it to make querying simpler, but I'm not sure if I've over/under done it.  The model tests fine, but it all feels a little clunky.

#####################################################################
# Base ##############################################################
class Base():
    def __init__(self, **kwargs):
        self._ClassName = self.__class__.__name__
        self._Created = datetime.now()
        self.__dict__.update(kwargs)

    def __repr__(self):
        return '<%s>' % self.__class__.__name__

#####################################################################
# Comment ###########################################################
class Comment(Base):
    def __init__(self, username, person, comment, parent, **kwargs):
        self.Username = username
        self.Person = person
        self.Comment = comment
        self.Parent = parent
        self.__dict__.update(kwargs)

    def _save(self):
        #save to Comments
        db.Comments.save(self.__dict__)
        #save to parent collection
        parent_obj = db.dereference(self.Parent)
        query = {'_id':parent_obj['_id']}
        update = {'$addToSet':{'Comments':self.__dict__}}
        db[parent_obj['_Collection']].find_and_modify(query, update, safe=True)
        #save to people collection
        query = {'_id':self.Person.id}
        db.People.find_and_modify(query, update, safe=True)

    def _remove(self):
        #remove from Comments
        query = {'_id':self._id}
        db.Comments.remove(query, safe=True)
        #remove from parent collection
        query = {'Comments._id':self._id}
        update = {'$pull':{'Comments':self.__dict__}}
        db[self.Parent.collection].find_and_modify(query, update, safe=True)
        #remove from people collection
        db.People.find_and_modify(query, update, safe=True)

    def __str__(self):
        return '%s - %s...' % (self.Username, self.Comment[:10])

person = {'Username':'foobarman'}
db.People.save(person, safe=True)

program = {'Title':'the foobar', '_Collection', 'Programs'}
db.Programs.save(program, safe=True)

comment = Comment(
                person['Username'],
                DBRef('People', person['_id']),
                'this is a comment about the foobar program',
                DBRef(program['_Collection'], program['_id']))

comment._save()

# find latest 10 comments
list(db.Comments.find().sort('_Created', -1).limit(10))

# find all comments for username
db.People.find_one({'Username':'foobarman'})['Comments']

# find all comments for program
db.Programs.find({'Title':'the foobar'})['Comments']

# find all comments for program by user
list(db.Comments.find({
                'Parent':DBRef(program['_Collection'], program['_id']),
                'Username':'foobarman'}))

#####################################################################
# Comment ###########################################################

This sort of stuff is useless noise that adds a lot of maintenance.

If your classes are getting hard to find, move them over to separate files. If you don't want to move them to separate files, consider making use of your IDE to find the class you're looking for, rather than relying on optical recognition via big blocks of screenfilling characters.

The reasons for this are twofold:

If you refactor the name of the class, the comment doesn't change and you spend time formatting a comment like this, when you could have been doing normal programming
If you ever had a name that is longer or wanted to describe a class with a docstring you might break convention and might have to restyle all your classes or risk inconsistencies


Whitelist HTML tags Microsoft Sanitizer and custom Regex I have the following code which I have written due to the fact the Microsoft's Sanitizer is now to aggressive.

What I'm trying to do is as follows.

Create a whitelist of HTML tags I want to keep
After the tags have been converted, run the text through the sanitizer and remove any tags I do not require.
After that, remove any tags that have either id's or classes inside them
Finally convert the tags back.

My code is below, and it would be much appreciated if someone with more experience can have a look over it and give me some feedback.

string txt = "
Keep as sanitizer removes

keep

remove class

";
            var whiteList = new List<Word>();
            whiteList.Add(new Word() { SearchWord = "
", ReplaceWord = "&lt;p&gt;" });
            whiteList.Add(new Word() { SearchWord = "
", ReplaceWord = "&lt;/p&gt;" });
            whiteList.Add(new Word() { SearchWord = "
", ReplaceWord = "&lt;h1&gt;" });
            whiteList.Add(new Word() { SearchWord = "
", ReplaceWord = "&lt;/h1&gt;" });
            whiteList.Add(new Word() { SearchWord = "
", ReplaceWord = "&lt;br/&gt;" });

            whiteList.ForEach(w => txt = txt.Replace(w.SearchWord, w.ReplaceWord));

            var remove = Sanitizer.GetSafeHtmlFragment(txt);

            whiteList.ForEach(w => remove = remove.Replace(w.ReplaceWord, w.SearchWord));

            var again = RegexHelpers.StripHtmlAttributes(remove);

            var tt = again;

public static string StripHtmlAttributes(string s)
    {
        const string pattern = @"\s.+?=[""'].+?[""']";
        var result = Regex.Replace(s, pattern, string.Empty);
    return result;
}

You could simplify the declaration of the tags you want to keep, as it is very structured:

var whitelist = new List<string>(new string[] { "p", "h1", "br" });

foreach (var w in whitelist) {
  txt = txt.Replace("<" + w + ">", "&lt;" + w + "&gt;")
           .Replace("<" + w + "/>", "&lt;" + w + "/&gt;"));
}

var remove = Sanitizer.GetSafeHtmlFragment(txt);

foreach (var w in whitelist) {
  remove = remove.Replace("&lt;" + w + "&gt;", "<" + w + ">")
                 .Replace("&lt;" + w + "/&gt;", "<" + w + "/>"));
}

var again = RegexHelpers.StripHtmlAttributes(remove);

var tt = again;


Checking for new software updates to the client I have written a very simple web service with MVC and WebApi. Now I'm working on the client code which will be a WPF application (and soon Windows 8 Store/Phone app). What I have done works, but I'm not sure I'm doing it the "right" way. The purpose of the service is to check if there are any new software updates to the client.

My server code looks like this (simplified):

public class ProductVersionsController : ApiController
{
    private ApplicationDbContext db = new ApplicationDbContext();

    [HttpGet]
    public CheckVersionResult CheckVersion(string product, string platform, string version)
    {
        CheckVersionResult result = new CheckVersionResult();

        //Logic removed...

        return result;
    }
}

My client code looks like this (simplified):

string parameters = "product=myproduct&platform=wpf&version=1.2.3.4";
string CheckUrl = "http://localhost:61933/api/ProductVersions/CheckVersion";

var url = new Uri(CheckUrl + "?" + parameters);
using (var client = new System.Net.WebClient())
{
    var json = await client.DownloadStringTaskAsync(url);

    CheckVersionResult data = JsonConvert.DeserializeObject<CheckVersionResult>(json);

    //Logic removed...
}

The deserializing is done with Json.net.

Should I use HttpGet for a service like this, or should I use Post to send the parameters? How the parameters are sent feels a bit clumsy. If the parameters were encapsulated in a class, how should that be solved on the client side? Is there any good practice I have missed?

Simplified, examplified, trimmed code is frowned upon on this site; I'm surprised this question hasn't received any close votes yet.

I don't see anything blatantly done wrong here - I like . On the other hand if the client code you've shown is cohesively written in a specialized service (class) with a clear, focused interface that your ViewModel  receives as a constructor parameter and assigns to a private readonly field, then you've probably done the wpf part right.

If that very same code is written in some Button1_Click handler in your View's code-behind, it could be the most cleverly written, beautifully crafted little piece of code, it would still be horribly misplaced.

</rant>

I still have a couple of things to say about your code:

private ApplicationDbContext db = new ApplicationDbContext();

The db identifier is misleading. I prefer to use context over db, because db refers to a database; a context, or data context, is more accurate. That said, by newing up your context inside your controller, you have tightly coupled your controller with the specific ApplicationDbContext class.

The Controller class implements IDisposable; you should override the Dispose method and properly dispose of your ApplicationDbContext instance.

An alternative would be to depend on an abstraction instead:

private readonly IDbContext context;

The dependency can then be constructor-injected, and the Controller doesn't care what the actual implementation is, it's simply calling methods defined by some interface.

This would greatly improve the controller's testability.

CheckVersionResult result = new CheckVersionResult();

I'm not a fan of this verbose notation; I find result is screaming its type at me, and I don't need that.

var result = new CheckVersionResult();

I find var makes more concise code that literally reads as "there's a variable here called result that we're going to assign to a new instance of the CheckVersionResult type" - as opposed to "there's a variable here of type CheckVersionResult called result that we're going to assign to a new instance of the CheckVersionResult type". But it boils down to personal preference I guess.

What isn't about personal preference, is consistency. Why is the client code different?

var url = new Uri(CheckUrl + "?" + parameters);
using (var client = new System.Net.WebClient())
{
    var json = await client.DownloadStringTaskAsync(url);

..and the next line goes:

CheckVersionResult data = JsonConvert.DeserializeObject<CheckVersionResult>(json);

I really prefer var I don't mind either notation, but using both interchangeably is wrong.

I like that you're using await. This means you've written this code in an async method. That method's name should return a Task or a Task<T>, and have an Async suffix in its name, to follow convention.


Containers and lists I want to create a container that has an AutoList object list. I wonder if my container is correct.

class Auto{
    string model;
    int co2;
public:
    Auto ();
    Auto(string m, int c);
    ~Auto(){};
    string GetModel(){return model;}
    void SetModel(string s){model = s;}
    int GetCo2(){return co2;}
    void SetCo2(int k){co2 = k;}
    bool operator < (const Auto &kitas);
};

class AutoList{
private:
    Auto data;
    AutoList *next;
public:
    AutoList(Auto a = Auto(), AutoList *al = NULL):
        data(a), next(al){}
    ~AutoList(){}

    void SetData(const Auto &a){data = a;}
    Auto GetData() const {return data;}

    void Set(AutoList *newaut){next = newaut;}
    const AutoList *Get() const {return next;}
    AutoList *Get(){return next;}
};

// I need to make a container class which saves AutoList object list.

class Container{ // is this class is container of AutoList object LIST???
private:
    AutoList *beg, *end, *d;
public:
    Container():beg(NULL), end(NULL), d(NULL){}
    ~Container(){Delete();}
    void Delete();

    Container(const Container & kitas);
    Container & operator = (const Container & kitas);

    void Begin(){d = beg;}
    bool True(){return d != NULL;}
    void Next(){d = d->Get();}
    AutoList GetAutoList(){return d->GetData();}
    void SetAutoList(const Auto & data);
};

void Container::Delete(){
    while(beg){
        d = beg;
        beg = beg->Get();
        delete d;
    }
    end = NULL;
}

void Container::SetAutoList(const Auto & data){
//..........
}

AutoList is for Node of Auto. The name AutoList is very confusing. I would name it as AutoNode so that a reader can know that it's a node element and not a List in itself.


Populating an HTML combo box by looped concatenation This code takes the results of a database query and uses them to populate the options in a combobox on a form.

Would it be better to start with an empty htmlCode and then concatenate all the <option> code in the loop before finally using echo outside the loop?

Also, do I need to declare $boxOption, $paramKey and $paramValue as class variables or are they OK as is because they're only used in this method?

public function populateComboBox ($boxToPopulate)
    {
        $this->boxOptions = $this->getQueryResults($this->boxParams[$boxToPopulate]['query']);

        foreach ($this->boxOptions as $boxOption) {

            foreach ($this->htmlParams as $paramKey => $paramValue) {

                if (!empty($this->boxParams[$boxToPopulate][$paramKey])) {

                    $this->htmlParams[$paramKey] = $boxOption[$this->boxParams[$boxToPopulate][$paramKey]];
                }
            }

            $this->htmlCode = "<option value='{$this->htmlParams['value']}' "
                              . "class='{$this->htmlParams['class']}'>"
                              . "{$this->htmlParams['text']}</option>\n";

            echo <<<_CODE
                                {$this->htmlCode}
_CODE;
        }

    }

I use an array for each combobox on the form:

    $this->boxParams['issue'] = array (
        'query' => 'SELECT interface_issue, current_issue '
                 . 'FROM interface_issue '
                 . 'WHERE current_issue = -1 '
                 . 'ORDER BY interface_issue;',
        'value' => 'interface_issue',
        'class' => '',
        'text' => 'interface_issue'
    );

You give us a very short piece of code, and two simple questions. I will not address the other issues of your code (or maybe I will), but concentrate on the two questions.

1. "Would it be better to start with an empty htmlCode and then concatenate all the  code in the loop before finally using echo outside the loop?"

I guess the other alternative is echo'ing the html inside the loop? This is probably a question about performance? The best way to figure this out, is to test both cases. If the question is about coding I would not echo inside this method at all, but just return a string with the HTML. This allow you to decide later, in another part of the code, what to do with the HTML: Store it? Put it in an email? Or echo it?

2. "do I need to declare $boxOption, $paramKey and $paramValue as class variables or are they OK as is because they're only used in this method?".

Well, if they're only used in this method, then they should be local variables, NOT the class variables which you made them into now. So replace $this-> by $.

Just one remark about your method: A method should have one purpose, one thing to do. Yours does several: 1. retrieve data from the database using a query that's stored in a strange way. 2. Build HTML, but not even a whole <select>, only the options. 3. Output the HTML. This is not very flexible. Your data always has to come from a predetermined place, and your output will always be echoed. Any variation on this method will require a completely new method, probably doing almost the same thing.

You also seem to have a class that does many things. The 'single responsibility principle' doesn't seem to have been applied. These principles are here for a very good reason. See:

http://en.wikipedia.org/wiki/SOLID_%28object-oriented_design%29

A programmer can ignore these principles for a long time, and programs will work, but when they grow, and expand, you will find that sticking to these principles can be very useful. Luckily the 'single responsibility principle' is one of the easiest to understand. A better explanation can be found here:

http://www.codemag.com/article/1001061


RSA calculating e in C# I would like the have a second opinion on my code I've written to calculate e

e is an integer that you wherefore the following counts gcd(φ(n),e) = 1 with 1 < e < φ(n)

public int calce()
{
    do
    {
        e = priemgen.newKey(1, euler);
    } while (GCD(euler, e) != 1);

    return e;
}

private int GCD(int value1, int value2)
{
    while (value1 != 0 && value2 != 0)
    {
        if (value1 > value2)
        {
            value1 %= value2;
        }
        else
            value2 %= value1;
    }

    return Math.Max(value1, value2);
}

private int NewPrime(int lowerbound, int upperbound)
{
    return priemgen.newKey(lowerbound, upperbound);
}

There is not a huge amount of code to review here, just a few things:

You have a method NewPrime which I assume provides a prime number between the two bounds provided. Yet you do not use it and call priemgen.newKey directly in calce. You should either get rid of the wrapper method or use it consistently.
Braces are used inconsistently:

else
    value2 %= value1;

Should be same as the if block:

else
{
    value2 %= value1;
}

The naming conventions are inconsistent. Standard naming convention for C# for methods and fields is PascalCase and local variables and parameters are camelCase:

calce -> CalculateE
newKey on priemgen should be NewKey
priemgen - I assume it's a field so should be PrimGenerator
lowerbound -> lowerBound
upperbound -> upperBound
GCD -> Gcd

If priemgen is what it sounds like then the name for nextKey should be NextPrime.
What does priemgen.newKey return if there is no prime within the given bounds?


Use linq to compute length of line defined as array of points I have an array of points:

 Point[] line; // { Point(1,1), Point(2,3), Point(1,1) }

Assuming Point has the relevant operators, what's the most linq-y way of computing the length of this line?

My best effort so far is:

float length = Enumerable.Range(0, line.Length-2)
    .Select(pointIndex => (line[pointIndex+1] - line[pointIndex]).Magnitude)
    .Sum();

Seems like there's a lot of subscripting in there though.

I would think there's a way to do something like this pseudocode:

float length = line.PairwiseMap(p1, p2 => (p2 - p1).Magnitude).Sum();

?

Another possible answer (first time I've ever suggested two answers) is to keep your existing approach, but modified just a little bit by using .Select()'s under-appreciated little brother that gives you access to an index variable:

float length = line.Skip(1)
    .Select((point, index) => (point - line[index]).Magnitude)
    .Sum();

Heck, speaking of overloads.  You can also keep your existing approach but use .Sum()'s overload:

Enumerable.Range(0, line.Length - 1)
    .Sum(pointIndex => (line[pointIndex + 1] - line[pointIndex]).Magnitude)


Realtime chat service I am new to Node.js and Socket.io. I implemented a realtime chat service that has a chatroom feature. When the actual product is launched, there will be at least 200 concurrent users with 50 groups of 4 people. I would appreciate any advice on its performance and any recommendations.

Server (remote server)

var express = require('express');
var app = express();
var http = require('http').Server(app);
var io = require('socket.io')(http);

app.use(express.static(__dirname + '/'));

//setup IO
io.on('connection', function(socket){
    console.log("A user is connected");
    var notice = {'content' : 'Hello World'};
    socket.emit("notice", notice);
    //listen for reply
    socket.on("reply",function(data){
        //extract group id, make sure the msg is sent to the particular group
        var groupid = data.group;
        var groupname = groupid + "chat";
        socket.emit(groupname, data);
        console.log("got msg");
        //save the data into mongo db
    });

    socket.on('disconnect', function(){
        console.log("A user is disconnected");
    });
});

//Open server
http.listen(3000, function(){
    console.log('listening on *:3000');
});

Client

<!DOCTYPE html>

  <meta charset="utf-8">
  <title>WebSockets Demo</title>

WebSockets Test

Entry

            <input id="entry" type="text">
            <input id="button" type="submit" value="Enter">

Your client JavaScript indentation is very, very messed up. I can hardly understand what's going on.

Some lines are missing an indentation, and some have an extra indentation, or even an extra space (which is extra very messed up, because you are leaving the alignment of a tab/indentation).

Everything inside the very first function should be indented one tab. Everything inside the socket.on's second parameter should be indented another tab. Everything inside $(#button).on's first parameter should be indented another tab.

Here is your tab code cleaned up:

$(function(){
    var groupid = 2; //this groupid will change in the actual app. Right now is hardcoded. At tun time, this roupid is passed from php.
    var socket = io("http://104.236.19.119:3000");
    socket.on('connect',function(){
        window.alert("You are connected.");
    });

    $('#button').on('click',function(){
        var entry = $("#entry").val();
        var reply_msg = {"content" : entry, "group":groupid};
        socket.emit('reply',reply_msg);
        $("#entry").val("");
    });
    var groupname = groupid + "chat";
    //got the msg and append to chat window
    socket.on( groupname , function(data){
        var new_msg = $("<p>" + data.content + "</p>");
        $('#msg').append(new_msg);
    });
})

This code is much, much easier to read.

$('#button').on('click',function(){
    var entry = $("#entry").val();
    var reply_msg = {"content" : entry, "group":groupid};
    socket.emit('reply',reply_msg); // <--
    $("#entry").val("");
});

You have a potential error here: in this function, you attempt to send a signal to the server that you are connected to. However, if the client had not yet connected to the server, socket.emit will fail.

To fix this, create a variable called isConnected and set it to false. Then, when the first socket.on fires you can set this variable to true. That way, you can setup an if statement in the code above so that you don't try to reply to the server without first connecting to the server.

Here is what I mean:

var isConnected = false;
socket.on('connect',function(){
    window.alert("You are connected.");
    isConnected = true;
});

$('#button').on('click',function(){
    if(isConnected) {
        var entry = $("#entry").val();
        var reply_msg = {"content" : entry, "group":groupid};
        socket.emit('reply',reply_msg);
        $("#entry").val("");
    } else {
        alert("You are not connected yet.");
    }
});


Fixed capacity unordered_map I have a need for a container that behaves like an unordered_map, but has a fixed capacity, and an eviction policy where the least recently inserted key-value pair is chosen to be evicted when an insertion is performed at capacity.

I've implemented this by creating a container which combines a regular unordered_map with a queue of iterators (implemented as a circular array). When a new item is inserted the iterator at the end of the queue is erased.

fixed_size_unordered_map

#include <array>
#include <unordered_map>

template<typename Key, typename U, std::size_t capacity>
class fixed_size_unordered_map {
    using iterator = typename std::unordered_map<Key, U>::iterator;
    using size_type = typename std::unordered_map<Key, U>::size_type;
    using value_type = typename std::unordered_map<Key, U>::value_type;

public:
    fixed_size_unordered_map() { iter_queue.fill(items.end()); }
    fixed_size_unordered_map(const fixed_size_unordered_map&) = default;
    fixed_size_unordered_map& operator=(const fixed_size_unordered_map&) = default;
    fixed_size_unordered_map(fixed_size_unordered_map&&) = default;
    fixed_size_unordered_map& operator=(fixed_size_unordered_map&&) = default;

    size_type count(const Key& key) const {
        return items.count(key);
    }

    const Key& at(const Key& key) const {
        return items.at(key);
    }

    std::pair<iterator, bool> insert(const value_type &v) {
        if(iter_queue[queue_idx] != items.end()){
            items.erase(iter_queue[queue_idx]);
        }
        const auto ret = items.insert(v);
        if(ret.second){
            iter_queue[queue_idx] = ret.first;
            queue_idx = (queue_idx + 1) % capacity;
        }
        return ret;
    }

    /* could implement more map<T, U> delegates as appropriate, the above are the ones
     * I actually need for my use case */

protected:
    std::unordered_map<Key, U> items;

    std::array<iterator, capacity> iter_queue;
    std::size_t queue_idx{0};
};

Example usage:

fixed_size_unordered_map<int, int, 10> m;
for(int i=0; i<=10; ++i){
    m.insert(std::make_pair(i, 2*i*i));
}
std::cout << m.at(10) << "\t" << m.at(1) << "\t" << m.count(0) << std::endl;

template<typename Key, typename U, std::size_t capacity>
class fixed_size_unordered_map {

The underlying map allows for policy customizations of the allocator, key comparator, and hash.  You may want to provide these customization points to users.

    using iterator = typename std::unordered_map<Key, U>::iterator;
    using size_type = typename std::unordered_map<Key, U>::size_type;
    using value_type = typename std::unordered_map<Key, U>::value_type;

Are these common type definitions intended to be private?

    fixed_size_unordered_map(const fixed_size_unordered_map&) = default;
    fixed_size_unordered_map& operator=(const fixed_size_unordered_map&) = default;
    fixed_size_unordered_map(fixed_size_unordered_map&&) = default;
    fixed_size_unordered_map& operator=(fixed_size_unordered_map&&) = default;

If you explicitly define any of the special member functions for a class, you must define them all.  The following five are considered special member functions. (see Rule of Zero/Three/Five)

Destructor
Copy Constructor
Copy Assignment
Move Constructor
Move Assignment

Either explicitly provide the missing destructor (Rule of Five) or don't explicitly define any of them (Rule of Zero).

    if(iter_queue[queue_idx] != items.end()){
        items.erase(iter_queue[queue_idx]);
    }
    const auto ret = items.insert(v);
    if(ret.second){

Undefined behavior here.  If the iter_queue is full, you erase no matter what happens. If insert succeeds, an iterator overwrites the current invalid iterator in the LRU.  If insertion doesn't succeed, nothing happens to the LRU and the current iterator is just invalid.

Check before erasing.  Erase when inserting.  Determine how you want the LRU to behave on failure to insert.

Consider using a circular buffer or writing your own LRU cache adaptor for some buffer type.  By decoupling the LRU functionality, this object can focus on just adapting its interface to the composed-from objects.  It also removes the need to explicitly define a default constructor (unless you define a conversion ctor).


Refactoring conditions in text parsing method I am writing a generic text parser and have come up with the following text parsing algorithm, which works fine. I'm wondering how to refactor the following method to make it easily understandable and maintainable for the future. It has chains of conditions and nested if-else blocks to compare with each text line to find matching criteria. Any ideas or comments would be highly appreciated ...

private void seekContents (String currentLine, String fontCurrentLine, String pdfContentPageNumber, String currentSearchToken) throws Exception

{
boolean isTokensToSkip=regCompare.isTokensToSkip(currentLine);
boolean isLinesToSkip=regCompare.isLinesToAvoid(currentLine);
boolean isTokensToStop=regCompare.isTokensToStop(currentLine);
boolean isKeywordToSeekAfterToken= regCompare.isKeywordPatternsToSeekAfterToken(currentLine);

boolean isKeywordPatternToSeekAfterTokenExists=regCompare.isKeywordPatternToSeekAfterTokenExists();

boolean hasFont=regCompare.hasLineFont(fontCurrentLine);
boolean isFileHeader=regCompare.isFileHeader(currentLine);
boolean isFileFooter=regCompare.isFileFooter(currentLine);

String pageNumber=regCompare.hasPageNumber(currentLine);
String hasContentPatternToSeekAfterToken = regCompare.hasContentPatternsToSeekAfterToken(currentLine);
String hasRegexTokensToSkip=regCompare.hasRegexTokensToSkip(currentLine);
String hasRegexTokenToStop=regCompare.hasRegexTokensToStop(currentLine);
String hasRegexTokenToAvoid = regCompare.hasRegexTokensToAvoid(currentLine);

if(isFileHeader||isFileFooter||isLinesToSkip)
{
    //System.out.println("Skipped the header/footer/lines to skip!");
}
else if(hasRegexTokenToAvoid!=null)
{
    //System.out.println("Skipped the Regex line!");
}

else if(pageNumber!=null)
{
    //System.out.println("Found the page#");
}
else if(isTokensToSkip)
{
    isSearchTokenFound=false;
}
else if(hasRegexTokensToSkip!=null)
{
    //System.out.println("Did i found regex stopper?");
    isSearchTokenFound=false;
}
else if(isTokensToStop)
{
    isSearchTokenFound=false;
    isParserEngineStop=true;
}
else if(hasRegexTokenToStop!=null)
{
    isSearchTokenFound=false;
    isParserEngineStop=true;
}
else
{
    //check for content pattern match

    if(hasContentPatternToSeekAfterToken!=null)
    {
        isContentPatternFound=true;

        if(isKeywordPatternToSeekAfterTokenExists)
        {
            contentPattern=hasContentPatternToSeekAfterToken;
            isKeywordAfterPatternFound=false;
        }
        else
        {
            keepRecordsNoKeywordPatternToSeekAfterTokenExists(hasContentPatternToSeekAfterToken,pdfContentPageNumber,currentSearchToken,currentLine);
        }
    }
    else
    {
        if(isContentPatternFound)
        {
            if(isKeywordPatternToSeekAfterTokenExists)
            {
                if(isKeywordToSeekAfterToken)
                {
                    keepRecordsKeywordToSeekAfterToken(contentPattern,pdfContentPageNumber,currentSearchToken,currentLine);
                    isKeywordAfterPatternFound=true;
                }
                else
                {
                    if(isKeywordAfterPatternFound)
                    {
                        desiredContents.add(currentLine);
                    }
                }
            }
            else
            {
                desiredContents.add(currentLine);
            }
        }
    }
}

}

As you probably suspected yourself,
this looks neither readable nor maintainable.

The method makes too many references to outside:
a dozen or so references in regCompare.*,
and a handful variables defined outside (isSearchTokenFound, isParserEngineStop, ...),
and a handful of other methods (keepRecordsNoKeywordPatternToSeekAfterTokenExists, keepRecordsKeywordToSeekAfterToken).

All this indicates that there is no good information hiding in your design.
Too many dependencies on external factors makes an implementation too complex and incomprehensible.
I'm afraid what you have here is effectively spaghetti code.

Fixing this is difficult. It's not unusual in software development that an approach turns into a complete mess and then you need to restart from scratch in a different direction. It can be very difficult to write a good readable parser.

Here are a few ideas to try to improve this code:

Eliminate anything unused: the hasFont variable is unused, and therefore the fontCurrentLine parameter too. Remove them.
Avoid unnecessary calls: if isFileHeader is true, then isFileFooter and isLinesToSkip won't have to be executed. It seems as if you're trying to declare all variables at the top for some reason. In general that's not a good practice. Try to declare and initialize variables right before they are really needed, no need to do far in advance.
Improve the names: hasRegexTokensToSkip sounds like a boolean, but it's actually a String
Early returns might help. The first couple of else-ifs look like guard statements. Sometimes, turning these into if + return might make the code somewhat easier to read by reducing the indent level of the last else block.

After you apply these simple transformations the code might look slightly better.
But in the end, you might be better starting over from scratch.


Is there any better way to fetch two tables in one statement or query? The 'Do' method is for prepared statements and returns (by fetchall()).  I am also using two looping statements.  Is there any better way to fetch two tables in one statement or query?  If not, what can I do to improve this code?

// 'id' is index in 'userinfo' table and 'id' in 'election' table is a child index(foreign key)
// 'ElectionId' is also the index, but it is not connected with othertable (only 'id' is linked)

 <?php
    $call = new world();
    $get = $call->Do("SELECT Votes, Views, id FROM election WHERE ElectionId = ?", array("electionIdUknwn"));
        foreach($get as $show)
            {

"Votes is :".$show[0]

                $add = $call->Do("SELECT username FROM userinfo WHERE id = ?", array($show[2]) );
                foreach($add as $display)
                {

"Username is :".$display[0]

                }

"Views are :".$show[1]

            }
  ?>

SQL:

This is just a simple join.

SELECT e.Votes, e.Views, e.id, u.username
FROM election as e, userinfo as u
WHERE e.id= u.id WHERE e.ElectionId=?
ORDER BY e.id ASC

Now you can iterate over all lines and every time e.id is changing you know that you have to display a new group.

Are there really multiple users per Votes and Views pair? I guess Votes and Views are counting columns so your model is kind of strange. Maybe you can find better names, so it is easier to get an idea what you want to do.

PHP:

usually classes are starting with a capital letter.
usually methods are starting with a lower case letter.
there is something missing in your output, some echos?
it is best practice to skip the ?> at the end, do avoid sending whitespace in case you will change the page header later.
$call and world are really strange names


Simpler version of capitalizing each word in a string I have this code which does the job, however it seems to me that the same could be done with less code. And why if I put 'var newString = stored.join(' ');' below 'var stored=[];' the code doesn't work?

function titleCase(str) {

var lower = str.toLowerCase();

var arr = lower.split(' ');

var stored = [];

//takes a string from arr
for(var i = 0; i<arr.length; i++) {

 //splits and stores the string to letters
   var word = arr[i].split(''); // ['l','i','t'...]

    // takes the first letter from the array and makes it uppercase
   for(var k=0; k<1; k++){
   var upper = word[k].toUpperCase();
   word.shift();
   word.unshift(upper);
   var joined = word.join('');
   stored.push(joined);
}

}
 var newString = stored.join(' ');

 return newString;

}

 titleCase("I'm a little tea pot");

You can use String#replace with a regexp:

function titleCase(str) {
  return str.toLowerCase() // change the string to lowercase
    .replace(/\S+/g, function(s) { // get all non space characters sequences
      return s.charAt(0).toUpperCase() + s.substring(1); // uppercase the 1st letter and combine with the rest of the string
    });
}

console.log(titleCase("I'm a little tea pot"));


Project Euler, Challenge #5 in Swift The problem:

2520 is the smallest number that can be divided by each of the numbers from 1 to 10 without any remainder.
What is the smallest positive number that is evenly divisible by all of the numbers from 1 to 20?

The solution in Swift:
import Foundation

var number = 2520
var found = false

while found == false {
    var counter = 0

    for var iterator = 2; iterator < 21; iterator++ {

        if number % iterator == 0 {

            counter++

        } else {

            println("\(number) is not the number")
            break;

        }

    }

    if counter == 19 {

        found = true;

    } else {

        number = number + 2

    }
}
println("\(number) is the number!!")

Comments on making this code more efficient, making it more cleaner or violations of best practices would be highly appreciated.

== false

while found == false

We'd prefer to see either this:
while !found

or this:
while notFound

(where in the second example, we reverse the boolean logic you've used)

Vertical White Space
A blank line between every line of code is not productive.  It adds wear to my scroll wheel and makes your code harder to digest.
Instead, we should think of blank lines as a way of taking a breather.  They should separate logical chunks of code within a function or method.

Single Responsibility
I'd guess that this is all simply dumped into a playground, which is fine.  But we can still write functions in the playground.  And we should.
We've got quite a lot of nesting.  Any time we have nesting is an indicator that we should be refactoring into functions.
Most importantly, functions have names.  Using functions allows us to write more readable code without using comments because we're breaking our code up into small logic chunks that make sense together and giving each of these chunks a name (the function name).
Also, breaking our code into these chunks makes sure we don't choke on the elephant.

;
Semicolons are optional in Swift.
Before anything else, we should be consistent.  Using semicolons on two lines and omitting on other lines has the potential to be confusing.  If you want to use semicolons, you need to use them everywhere.
But this is Swift, and in Swift, we prefer omitting semicolons.  I don't use semicolons anywhere in Swift.

Traditional for Loops.
We should basically never use these in Swift.  I can't think of any scenario which requires their usage.  First of all, from a style and readability standpoint, this is actually harder to read than the Swift alternatives, which make more sense from a plain-English point of view.  The only reason the traditional for loops are even remotely readable is because of the decades of history that the C language has dumped on us.
We should vastly prefer something more like this:
for iterator in 2...20 {
    // do something with 2 through 20 inclusive...
}

And beyond the style comment, these Swift loops are actually faster than the traditional for loops.  (And this explains how I manage to never use semicolons in Swift.)

var versus let
var declares a variable.  let declares a constant.
We should always prefer let to var.  The only time we should use var is when we're confident we will need to modify the value.  Very few of your values need to be modified and most can be declared as let values.

Magic Numbers
Your code is littered with magic numbers.  Rather than magic numbers, we should declare constants and name all of these numbers.  Although, if we properly broke our code up into functions, most of these magic numbers would go away in place of function input arguments.
The only magic number in here we can't really get rid of is 0.  Any time a number's best variable name would simply be the name of that number, it's not really a magic number.  The only number for which this actually applies in your code is 0.
However, with that said, we can take care of zero with a function in this case:
extension Int {
    func isMultipleOf(divisor: Int) -> Bool {
        return self % divisor == 0
    }
}

We still have 0 in here, but it's wrapped inside a very small, very concise, very aptly named function that describes exactly what's going on here.

In summary...
Of course, this answer hasn't addressed any performance concerns.  I've only addressed style concerns.  I think style is one of the most important things to get right very early, before you've dug yourself into lots of bad habits.
For examples on what this might look like both in terms of better style and better performance, you can check out my question regarding this very same Project Euler challenge.
I'll be glad to help you with performance issues, but others have already addressed this, and I wouldn't really be able to say anything besides "go look how I did it, plus the performance-related answers I got".  I wanted to emphasize Swift-style however, because I'm not sure how likely it is that any performance-related comments will be applicable to the next Swift code you write, but all of these style-related comments will absolutely be applicable.
And the better your style it is, the easier it is for both yourself and any reviewer to look at and focus on performance issues.


Less CSS Mixin for positioning with options for corresponding top, left, right, bottom values I've created a Mixin in Less CSS. It can do the following things:

Set the position-rule of an element.
Place the element in one of the corners (e.g. top left, bottom right etc.)
Alternatively it can set explicit values for top|right|bottom|left.

Usage example:

.test-1 {
    .position(absolute, top, right);
}

Examples showing the resulting CSS:

.position() { position: static; }
.position(relative) { position: relative; }
.position(absolute, top, right) { position: absolute; top: 0; bottom: auto; left: auto; right: 0; }
.position(absolute, bottom, left) { position: absolute; top: auto; bottom: 0; left: 0; right: auto; }
.position(fixed, 20px, 50%) { position: fixed; top: 20px; right: 50%; }
.position(fixed, 10px, null, null, 10%) { fixed: absolute; top: 10px; left: 10%; }
.position(absolute, 10px, -, -, 10%) { position: absolute; top: 10px; left: 10%; }

The Mixin:

.position(@position: static; @a: null; @b: null; @c: null; @d: null) {
    position: @position;

    & when (top = @a) {
        top: 0;
        bottom: auto;
    }

    & when (bottom = @a) {
        top: auto;
        bottom: 0;
    }

    & when (left = @b) {
        left: 0;
        right: auto;
    }

    & when (right = @b) {
        left: auto;
        right: 0;
    }

    & when not (null = @a) and not (~'-' = @a) and not (top = @a) and not (bottom = @a) {
        top: @a;
    }

    & when not (null = @b) and not (~'-' = @b) and not (top = @b) and not (bottom = @b) {
        right: @b;
    }

    & when not (null = @c) and not (~'-' = @c) {
        bottom: @c;
    }

    & when not (null = @d) and not (~'-' = @d) {
        left: @d;
    }
}

I've also created a shortahand version .p(), which can be called instead of .position():

.p(@position: static; @a: null; @b: null; @c: null; @d: null) {
    .position(@position; @a; @b; @c; @d);
}

My questions are:

Is this a useful Mixin?
Or is it a bad practie to create Mixins like this?
Is it too complicated or too hard to understand?
Can this be optimized or shortend?

Even though most of the time I need to use top and left for positioning, I found it more related to other CSS rules like margin when the values are sorted clockwise like top|right|bottom|left.

Is this a useful Mixin? Depends on if you need it a lot.

Or is it a bad practice to create Mixins like this? Nothing is bad practice if it helps you float your boat.

Is it too complicated or too hard to understand? Not really, but seems like you'll often have to lookup the generated CSS to fix positioning problems. F.e. I'm not a big fan of 10px, null, null, 10%, I know it's NESW, but still... Wouldn't it be better to have .position(fixed, top 10px, left 10%) if that is possible?

Can this be optimized or shortened? Probably yes, but why do premature optimization if the use-case is unknown?


MS CA2202: Object can be disposed more then once When running Code Analysis on my project I get the following warning:

  CA2202 : Microsoft.Usage : Object 'fs' can be disposed more than once in method

I don't see the problem, and the code compiles fine and produces the expected results in all test cases.
If I remove the fs.Dispose(); statement then Code Analysis instead produces this warning:

  object 'fs' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'fs' before all references to it are out of scope.

I am much grateful if you can review my code so that I can learn the proper way of disposing resources.

    private string ReadFileContent(string filename, Encoding encoding, string category, FileArchive archiveConfig)
    {
        FileStream fs = null;
        StreamReader sr = null;
        string fileContent = string.Empty;

        _logger.Debug("Reading file content of \"" + category + "\" file \"" + filename + "\"");

        try
        {
            fs = new FileStream(filename, FileMode.Open, FileAccess.ReadWrite, FileShare.None, 64 * 1024, FileOptions.SequentialScan);
            fs.Position = 0;
            sr = new StreamReader(fs, encoding, true);
            fileContent = sr.ReadToEnd();
        }
        catch (Exception)
        {
            ArchiveFile(filename, archiveConfig.ErrorFolder, category, _logger);
            throw;
        }
        finally
        {
            if (sr != null)
                sr.Dispose();
            if (fs != null)
                fs.Dispose();
        }

        return fileContent;
    }

As @amon mentioned, the correct way of referencing IDisposable Objects in C# is the using-statement.

In general you then have the try-catch block around the using statement to catch eventual initialization errors. This means your Code would look somewhat like this:

String fileContent = String.Empty;
_logger.Debug("Debug-String");

try{
   using(FileStream fs = new Filestream(filename, FileMode.Open, FileAccess.ReadWrite,
     FileShare.None, 64 * 1024, FileOptions.SequentialScan){
      fs.Position = 0;
      using (StreamReader sr = new Streamreader(fs, encoding, true){
         fileContent = sr.ReadToEnd();
      }
   }
}
catch (Exception){
   ArchiveFile(filename, archiveConfig.ErrorFolder, category, _logger);
   throw;
}

return fileContent;

This also allows you to declare variables only when used.

Finally a small note on filename... you are using camelCase in all variables but that. Be consistent in your naming schema


First Unique Character in a String The task

is taken from leetcode

  Given a string, find the first non-repeating character in it and
  return it's index. If it doesn't exist, return -1.

  Examples:

  s = "leetcode"

  return 0.

  s = "loveleetcode",

  return 2.

  Note: You may assume the string contain only lowercase letters.

My solution

/**
 * @param {string} s
 * @return {number}
 */
function firstUniqChar(s) {
  const arr = Array.from([...s]
                         .reduce((map, x, i) => map.set(x, !isNaN(map.get(x)) ? null : i) , new Map())
                         .values());
  for(let i = 0; i < arr.length; i++) {
    if (arr[i] !== null) { return arr[i]; }
  }
  return -1;
};

Your solution looks rather map-happy.

It is important to remember for this task that counting higher than 2 of any encountered letter is needless processing as is any processing on any letter after the earliest positioned unique letter.

Also, doing a complete sweep of the input string may be ill-advised if the input string is of considerable length.

Due to not being as well across .js as others here are, I'll post a humble for loop.

function firstNonRepeatedCharacterPosition(string) {
    for (let char, pos, i = 0; i < string.length; ++i) {
        char = string.charAt(i);
        pos = string.indexOf(char);
        if (pos == i && string.indexOf(char, i + 1) == -1) {
            return pos;
        }
    }
    return -1;
}

console.log(firstNonRepeatedCharacterPosition('abcbebc'));

It does make 3 function calls per iteration, but they aren't heavy ones, there is at most only one pass through the string, and early-return programming is in effect.

Grab the letter at the incremented position.
Find the earliest occurrence of that letter.
Check if there is a later occurrence of the same letter.

The later the unique letter exists (or if there are no unique letters), the more laborious my function is.  On the other hand, if the first letter is unique, then you are finished in just 3 function calls.

p.s. I lack the knowledge to interpret your js code, so I cannot review it beyond saying that it isn't very novice friendly.

After Rotora's challenge, I was doubting myself, so I whacked this little battery of tests together:

function MickMacKusa(string) {
    for (let char, pos, i = 0; i < string.length; ++i) {
        char = string.charAt(i);
        pos = string.indexOf(char);
        if (pos == i && string.indexOf(char, i + 1) == -1) {
            return pos;
        }
    }
    return -1;
}

function RoToRa(string) {
    for (let char, pos, i = 0; i < string.length; ++i) {
        char = string.charAt(i);
        if (string.indexOf(char, i + 1) == -1) {
            return i;
        }
    }
    return -1;
}

let table = document.getElementById("test"),
    row;
for (let i = 1; i < table.rows.length; ++i) {
    row = table.rows[i];
    row.cells[3].innerHTML = MickMacKusa(row.cells[0].innerHTML);
    row.cells[4].innerHTML = RoToRa(row.cells[0].innerHTML);
}

Input	Letter	Index	MickMacKusa	RoTora

abccbcba	  -	 -1

abcbebc	   a	  0

abc	       a	  0

aabbc	     c	  4

abcba	     c	  2

abba	      -	 -1

abaa	      b	  1

aabcbcbca	 -	 -1


Duplicating Excel-workbook, and transferring data This is a follow up to the simpler question here.

I have hundreds of files looking something like this:

These files have names such as:

2301 Item 1.xlsx
2014 Item 2.xlsx
5321 Item 3.xlsx
3212 Item 200.xlsx

I also have a template file (Template.xlsx) looking like this:

I want to:

Copy the template file, and for each of the other files

Give it the filename in cell C6.
Copy the values of B2,B3,B3 over to the appropriate spot in the template (B4,B5,B6).
Copy the filename to B8

I've written the following script in Python 3:

import os
import shutil

from xlrd import open_workbook
from xlutils.copy import copy

def duplicate_and_transfer(path):

    for filename in os.listdir(path):
        if filename == 'Template.xlsx':
            continue
        else:
            filepath = os.path.join(path, filename)
            excel_file = open_workbook(filepath)
            worksheet = excel_file.sheet_by_index(0)
            new_filename = worksheet.cell_value(5,2)
            new_filepath = os.path.join(path, new_filename + '.xls')
            r_value = [0,0,0]
            for row in range(3):
                r_value[row] = worksheet.cell_value(row+1,1)

            shutil.copy2(os.path.join(path, 'Template.xlsx'),new_filepath)
            rb = open_workbook(new_filepath)
            wb = copy(rb)
            s = wb.get_sheet(0)
            s.write(7,2,new_filename)
            for r in range(3):
                s.write(r+3,1,r_value[r])

            wb.save(new_filepath)

if __name__ == '__main__':
    duplicate_and_transfer(r'C:\Users\Stewie\Documents\excel_folder')

A few things:

I have to make the new files xls instead of xlsx. They get corrupted if not (someone else have had this problem). Can I avoid this?
Should I split this up into more functions?

This is fast enough, so my main issue is not with performance.

Since you use continue in the if clause, you can get rid of the else part and save a level of indentation.
You can use a list-comprehension to build the list of r_value: [worksheet.cell_value(row + 1, 1) for row in range(3)].
You can iterate directly over the content of r_value to set it back:

for row, value in enumerate(r_value, 3):
    s.write(row, 1, value)

I don't think you need to copy the template first and modify it after. You may be able to open the template and then save it using the new filename.
You can use a second parameter with a default value to hold the name of the template.

Proposed improvements:

import os

from xlrd import open_workbook
from xlutils.copy import copy

def duplicate_and_transfer(path, template_name='Template.xlsx'):
    template = os.path.join(path, template_name)

    for filename in os.listdir(path):
        if filename == template_name:
            continue

        filepath = os.path.join(path, filename)
        excel_file = open_workbook(filepath)
        worksheet = excel_file.sheet_by_index(0)
        new_filename = worksheet.cell_value(5,2)
        new_filepath = os.path.join(path, new_filename + '.xls')
        values = [worksheet.cell_value(row + 1, 1) for row in range(3)]

        rb = open_workbook(template)
        workbook = copy(rb)
        worksheet = workbook.get_sheet(0)
        worksheet.write(7, 2, new_filename)
        for row, value in enumerate(values, 3):
            worksheet.write(row, 1, value)

        workbook.save(new_filepath)

if __name__ == '__main__':
    duplicate_and_transfer(r'C:\Users\Stewie\Documents\excel_folder')

You may as well be able to open the template once and save it several times.

Lastly, if you have troubles working with XLSX files, you may want to try openpyxl instead:

import os

from openpyxl import load_workbook

def duplicate_and_transfer(path, template_name='Template.xlsx'):
    template = load_workbook(os.path.join(path, template_name))

    for filename in os.listdir(path):
        if filename == template_name:
            continue

        workbook = load_workbook(os.path.join(path, filename))
        worksheet = workbook.worksheets[0]

        new_filename = worksheet['C6'].value
        template['B8'] = new_filename
        new_filepath = os.path.join(path, new_filename + '.xlsx')

        # Using double unpacking as the ranges will return a 3-tuple of 1-tuple
        for (src,), (dest,) in zip(worksheet['B2:B4'], template['B4:B6']):
            dest.value = src.value

        template.save(new_filepath)

if __name__ == '__main__':
    duplicate_and_transfer(r'C:\Users\Stewie\Documents\excel_folder')


Generate key from array What do you think about quality of code in this method?

It should:

Generate new key based on keys in array
The key should start with "TG"
Code should find the max number in array keys with "TG-" and increment this number
It should have 7 digits
No need to check what if there is a key TG9999999, and what if array is empty or doesn't have keys "TG-"

private static string[] allKeys = new string[]
{
    "TG0000006",
    "TG0000026",
    "TG0000086",
    "TG0000796",
    "TG0023106",
    "LG0004406",
    "MS0000796",
    "TT0023106",
    "LK0004406",
};

static string GenerateKey()
{
    var keys = allKeys.Where(k => k.StartsWith("TG"));
    List<int> nums = new List<int>();
    foreach (var key in keys.Select(str => Regex.Match(str, @"\d+").Value))
    {
        int num;
        if (int.TryParse(key, out num))
            nums.Add(num);
    }
    string newNum = (nums.Max() + 1).ToString();
    return "TG" + new string('0', 7 - newNum.Length) + newNum;
}

If you want only the numbers 0..9 to be matched by the Regex and you don't care about for instance some Eastern Arabic numerals like ´٠١٢٣٤٥٦٧٨٩, then you should use an expression like [0-9]+.
because by using either the d+ or [0-9]+ expression you already made sure that the var key is a number. Because this number has at the maximum 7 digits, you can simply call Parse() instead of TryParse() which eleminates the usage of the int num.
please add always braces {} although they might be optional. This will make your code less error prone.
the return statement

return "TG" + new string('0', 7 - newNum.Length) + newNum;

can be simplified by using the PadLeft() method like so

return "TG" +  newNum.PadLeft(7,'0');

personally I would just have a if clase for checking the highest number instead of the List<int> together with the call to Max().

Putting this all together leads to

static string GenerateKey()
{
    int max = int.MinValue;
    var keys = allKeys.Where(k => k.StartsWith("TG"));
    foreach (var key in keys.Select(str => Regex.Match(str, @"[0-9]+").Value))
    {
        int num = int.Parse(key);
        if (num > max)
        {
            max = num;
        }
    }

    return "TG" + (max + 1).ToString().PadLeft(7, '0');
}


Using binary search to find the index of an item in a sequence Here's my script:

def index(seq, item):
    """Returns the index of an item in an increasingly-sorted
       sequence (if exists) otherwise returns the index where
       it would be inserted, using binary search.

       seq: a sequence.

       precondition: seq is sorted increasingly.
       """
    # Updates the sequence with the item using a
    # set to avoid duplicates.
    seq = set(seq)
    seq.update([item])

    # Converts the sequence to a list and re-sorts it.
    sorted_seq = sorted(seq)

    middle = len(sorted_seq) // 2
    middle_value = sorted_seq[middle]

    first_half = sorted_seq[:middle]
    second_half = sorted_seq[middle:]

    if item == middle_value:
        return middle

    elif item < middle_value:
        return index(first_half, item)

    elif item > middle_value:
        return index(second_half, item) + len(first_half)

How can it be refactored and optimized?

Notes:

I'm a hobbyist and beginner (in programming in general, and Python in particular).
I'm new to binary search I just understand the basic idea.

seq = set(seq)

This will cause index calculations to be incorrect. Consider this invocation:

index([1, 1, 2, 3], 2)

This should return 2 but your function returns 1. As a caller of your index method, I would expect the returned index to have item in seq.

    seq.update([item])

This is a bug. It's causing print(index([1, 1, 2, 3], 15)) to return 3, when it should return -1 or some other sentinel value to denote absence of an item in seq.

EDIT: The function's documentation clearly says that index will return the final index after insertion if item does not exist in seq.

sorted_seq = sorted(seq)

This is unnecessary. Validate/assume your input seq to be sorted. This should be the standard expectation from any caller invoking your index function.

Be careful about:

first_half = sorted_seq[:middle]
second_half = sorted_seq[middle:]

Python fortunately will not create copies of the list but other languages may create copies of the original list for this operation.

After you have removed seq.update([item]), you will need to check for the base of your recursion, namely when seq is empty, causing

middle_value = sorted_seq[middle]

to fail.

As an aside, try implementing this as an iterative algorithm.

It will run faster
The recursive version will crash for large seq.

Totally aside, have a look at Binary Search tutorial on TopCoder Its a pretty good article.


Can I adjust this code which displays a table to run faster? this.displayData = function () {
    var i;
    var $thead = $(thead);
    var $tbody = $(tbody);
    var numberOfFields = columnDataFields.length;
    var numberOfRows = data.length;

    // Variables to keep track of even and odd rows.
    var evenRowClass = 'EvenBar';
    var oddRowClass = 'OddBar';
    var t = false;

    $tbody.html('');

    // Find the widths of the column headers and apply the same widths here.
    // However, only do this if the colWidths are thus far undefined.
    if (!colWidths) {
        colWidths = [];
        var headers = $thead.find('td');
        var colWdith;
        for (i = 0; i < headers.length; i++) {
            colwidth = $(headers[i]).outerWidth();
            colWidths.push(colwidth);
            $(headers[i]).css('width', colwidth); // This statement is necessary for cross-browser compatibility. (The problem is with discrepencies in "computed" CSS.)
        }
    }
    for (x in data) {
        var entry = data[x];
        var row = ['
');
        for (i = 0; i < numberOfFields; i++) {
            row.push('');
            row.push(entry[columnDataFields[i]]);
            row.push('	');
        }
        row.push('
');

        $tbody.append(row.join(''));
        t = !t;
    }

    // If a callback function was set, invoke it.
    if (typeof displayCallback === 'function')
        displayCallback();
} // end displayData

You don't need to redraw the entire table. Try just doing DOM manipulation to reorder the TRs.  I see that you also want to stripe the rows, but you can still do it with DOM manipulation. You should be able to take it from there.

I created a simple example with almost 1000 rows and it runs almost instantaneously. There's some jQuery just so I don't have to worry about cross-browser issues. If you don't use jQuery, most libraries have the functionality I used,or you can just implement $.text on your own http://jsfiddle.net/unffs/2

var table = document.getElementById('tbl');
// Can't call sort on an HTML collection, so copy the contents to an array
var rows = Array.prototype.slice.call(table.rows);
rows.sort(function(a, b) {
    var aVal = $(a).text(), bVal = $(b).text();
    return aVal > bVal ? 1 : (bVal > aVal ? -1 : 0);
});
var tbody = document.getElementById('tbody');
for (var i=0; i < rows.length; i++) {
    tbody.appendChild(rows[i]);
}

That code works for a table like the following

D

A

C

E

F

B

D

Using data to sort instead of HTML

If you have some underlying data for the table, it will be even faster since you won't need to read the DOM (except for an id attribute)

var data  = [
   {id: 1, value: 'C'},
   {id: 2, value: 'B'},
   {id: 3, value: 'D'},
   {id: 4, value: 'A'}
];

And your HTML looks like

C

B

D

A

Here's the code to sort the table

var table = document.getElementById('tbl');
// Can't call sort on an HTML collection, so copy the contents to an array
data.sort(function(a, b) {
    return a.value > b.value ? 1 : (b.value > a.value ? -1 : 0);
});
var tbody = document.getElementById('tbody');
for (var i=0; i < data.length; i++) {
    tbody.appendChild(document.getElementById('row-' + data[i].id));
}


Asynchronous iterator for AngularJS I wrote this as a convenient way to batch together multiple, repetitive AJAX calls. I was working in angular at the time, so I use the $q service, but I'm sure it could be adapted for a more general promise framework. I find it so useful, I'm interested in polishing it for general consumption.

// A utility function that can run for-like loops with asynchronous processes.
// Use just like forEach, but you can launch something like an ajax call in the
// loop, as long as you return a promise. You can choose to run the calls in
// serial (one after the other) or in parallel (launching them all at once).
// Either way, you can attach a callback when they're all done, and an array of
// the promise resolve values of each step is available.
// For example, this would call doAsyncJob() for every item in myList, one
// after the other, and run doSomethingElse() once those were all done:

// $forEachAsync(myList, function (item) {
//     return doAsyncJob(item);
// }, 'serial').then(function (arrayOfJobResults) {
//     doSomethingElse();
// });
MyAngularApp.factory('$forEachAsync', function ($q) {
    'use strict';
    return function forEachAsync(arrayOrDict, f, serialOrParallel) {
        if (serialOrParallel === 'parallel') {
            // Iterate over the data, calling f immediately for each data
            // point. Collect all the resulting promises together for return
            // so further code can be executed when all the calls are done.
            return $q.all(forEach(arrayOrDict, f));
        }
        if (serialOrParallel === 'serial') {
            // Set up a deferred we control as a zeroth link in the chain,
            // which makes writing the loop easier.
            var serialDeferred = $q.defer(),
                serialPromise = serialDeferred.promise,
                returnValues = [];
            // Do NOT make all the calls immediately, instead embed each data
            // point and chain them in a series of `then`s.
            // Note: this makes the assumption that forEach iterates over both
            // arrays and objects by providing only two arguments.
            forEach(arrayOrDict, function (a, b) {
                serialPromise = serialPromise.then(function (value) {
                    returnValues.push(value);
                    return f(a, b);
                });
            });
            // Fire off the chain.
            serialDeferred.resolve();
            // Return the whole chain so further code can extend it, making
            // sure to return the resolve values of each iteration as a list.
            return serialPromise.then(function (value) {
                // Add the final resolve value, and slice off the first, which
                // is the "zeroth link" value and is always undefined.
                returnValues.push(value);
                return returnValues.slice(1);
            });
        }
        throw new Error(
            "Must be 'serial' or 'parallel', got " + serialOrParallel
        );
    };
});

Regarding the desire to make an implementation of the $q service, it is useful to consider that angular's $q is a stripped down version of the excellent promise library q by, kriskowal which you can leverage independently of angular. You can also achieve it through other promise implementations such as bluebird and with native ES2015 promises where available.

As for the implementation itself, I strongly recommend removing the serialOrParallel and doing one of the following.

Turn you function into an object which exposes a parallel and a sequential property. Ex:

var forEach = {
    parallel: function () {...},
    sequential: function() {...}
}

Create 2 separate functions, one that traverses sequentially and one that traverses in parallel.

The reasoning behind this is that it makes the code clearer, removes the need for an exception, and makes it easier for consumers to do the right thing.

As @Roamer-1888 noted, In the serial case, it should be possible to construct returnValues in its entirety in the forEach loop without post-processing it.


Minimizing open-close scenario I have the following code in Python which opens and closes a file in every iteration (using with keyword). In 95 % of iterations the file will be the same, so there's no need to open and close it again. Is it possible to optimize it if this optimization has sense?

import sys
import re

def createFileName(str, index, number_of_words, suffix):
  pattern = re.compile("[a-zA-Z]+")
  matchList = re.findall(pattern, str)
  matchList = [x.lower() for x in matchList]
  matchList = matchList[0:number_of_words]
  return index + "_".join(matchList) + suffix

if __name__ == "__main__":

  d = [("aaa", True), ("bbb x yyy aa" False), ("ccc aa", False), ("ddd bbb", True), ("eee ddd", False), ("fff x a b" True)] # this list of tuples is just for illustration, it will be usually much bigger.

  fileName = createFileName(d[0][0], "00", 3, ".txt")
  with open(fileName, "a") as f:
    f.write(d[0][0] + "\n")
  i = 1
  for x in d[1:]:
    if x[1]:
      fileName = createFileName(x[0], "%02d" % i, 3, ".txt")
      i = i + 1
    with open(fileName, "a") as f:
      f.write(x[0] + "\n")

I think this is one of those rare case where with hits its limits and you should be better served with a try: ... finally.
You can open the first file, and wraps the rest of the operations into a
try:
    # rest of the operations
finally:
    f.close()

Then, each time the fileName need to change, you f.close() and f = open(...) again. This way, you only close the file when you won't use it anymore.
As for the rest of the code:

use 4 spaces per indentation level
use snake_case for variable and functions names
use unpacking to extract and give a meaningful name to the 2 components of the items in your list
create pattern as a global variable as there is no added value to compile it each time you call createFileName
you can avoid copying d when you slice it if you use iter

Proposed improvements
import sys
import re

PATTERN = re.compile("[a-zA-Z]+")

def create_file_name(string, index, number_of_words, suffix):
    matches = [x.lower() for x in re.findall(PATTERN, string)]
    return '{:02}{}.{}'.format(index, '_'.join(matches[:number_of_words]), suffix)

if __name__ == "__main__":
    d = [("aaa", True), ("bbb x yyy aa" False), ("ccc aa", False), ("ddd bbb", True), ("eee ddd", False), ("fff x a b" True)] # this list of tuples is just for illustration, it will be usually much bigger.

    iterator = iter(d)
    file_index = 0
    name, _ = next(iterator)
    filename = create_file_name(name, file_index, 3, 'txt')
    f = open(filename, 'a')
    try:
        f.write(name + '\n')
        for name, new_file in iterator:
            if new_file:
                file_index += 1
                filename = create_file_name(name, file_index, 3, 'txt')
                f.close()
                f = open(filename, 'a')
            f.write(name + '\n')
    finally:
        f.close()


Polling timer function in C I've implemented a polling function with a timer in C, that every 10s checks a given condition (I've just replaced it for a log to stdout for testing purposes only) but I would like to know your opinion on this code, namely if there is a simpler way to achieve the same result, what issues may arise concerning performance, thread-safety, memory footprint. I'm interested in writing the most simple, correct and fast code for this. Suggestions are, as always, welcome.

/* includes removed for clarity */

void signalAlarmHandler(int signum) {
  switch(signum) {
  case SIGINT:
    fprintf(stdout, "received interrupt. going to exit.\n");
    exit(0);
    break;
  case SIGALRM:
    fprintf(stdout, "received signal alarm.\n");
    /* checking for condition here */
    break;
  default:
    break;
  }
}

int main(int argc, char* argv[]) {

  if(initializeEnvironment(argc, argv) == -1) {
    return -1;
  }
  else {

    signal(SIGINT, signalAlarmHandler); /* for testing purposes */
    signal(SIGALRM, signalAlarmHandler);

    fprintf(stdout, "starting the signal alarm stuff\n");
    while(1) {
      alarm(10);
      sleep(20);
    }
  }
}

What is a reason for using alarm() with sleep()? Why not just

void sigIntHandler(int signum) {
  fprintf(stdout, "received interrupt. going to exit.\n");
  exit(0);
}

int main(int argc, char* argv[]) {
  if(initializeEnvironment(argc, argv) == -1) {
    return -1;
  }
  signal(SIGINT, signalAlarmHandler); /* for testing purposes */

  while(1) {
    sleep(10);
    /* checking for condition here */
  }
}

Also:

fprintf() is not signal-safe, better not to use it from signal handler
signal() is obsolete and implementation-dependent, use sigaction() for consistent results.


Shared counter variable among different threads I have a need to use a shared variable among different threads in c/C++.
So putting my knowledge of threads and mutexes I have written the below sample code.
Can anyone please review the code and provide me review comments so that I can improvise it in a more better way. Also I know about atomic variables but somehow I don't want to get into that and wanted to stick with threads & mutexes and locking mechanism. Below code is a working code.

#include <iostream>
#include <unistd.h>
#include <pthread.h>

using namespace std;

class Thread
{
  public:
    Thread();
    virtual ~Thread();

    int start();
    int join();
    int detach();
    pthread_t self();

    virtual void* run() = 0;

  private:
    pthread_t  m_tid;

    int        m_running;
    int        m_detached;
};

static void* runThread(void* arg)
{
    return ((Thread*)arg)->run();
}

Thread::Thread() : m_tid(0), m_running(0), m_detached(0) {}

Thread::~Thread()
{
    if (m_running == 1 && m_detached == 0) {
        pthread_detach(m_tid);
    }
    if (m_running == 1) {
        pthread_cancel(m_tid);
    }
}

int Thread::start()
{
    int result = pthread_create(&m_tid, NULL, runThread, this);
    if (result == 0) {
        m_running = 1;
    }
    return result;
}

int Thread::join()
{
    int result = -1;
    if (m_running == 1) {
        result = pthread_join(m_tid, NULL);
        if (result == 0) {
            m_detached = 0;
        }
    }
    return result;
}

int Thread::detach()
{
    int result = -1;
    cout<<"Detaching thread"<<endl;
    if (m_running == 1 && m_detached == 0) {
        result = pthread_detach(m_tid);
        if (result == 0) {
            m_detached = 1;
        }
    }
    return result;
}

pthread_t Thread::self() {
    return m_tid;
}

class Mutex
{
    friend class CondVar;
    pthread_mutex_t  m_mutex;

  public:
    // just initialize to defaults
    Mutex() { pthread_mutex_init(&m_mutex, NULL); }
    virtual ~Mutex() { pthread_mutex_destroy(&m_mutex); }

    int lock() { return  pthread_mutex_lock(&m_mutex); }
    int trylock() { return  pthread_mutex_trylock(&m_mutex); }
    int unlock() { return  pthread_mutex_unlock(&m_mutex); }
};

class shared_integer {
private:
    int i;
    Mutex mlock;

public:
    // Parameterised constructor
    shared_integer(int i = 0)
    {
        this->i = i;
    }
    // Overloading the postfix operator
    void operator++(int)
    {
       mlock.lock();
       (this->i)++;
       mlock.unlock();
    }

    void operator--(int)
    {
         (this->i)--;
    }

    // Function to display the value of i
    void display()
    {
        cout << "Value became:" <<i << endl;
    }
};
class MyThread:public Thread
{
Mutex mlock;
shared_integer &wd;
public:
MyThread(
         shared_integer & workd
        ):
         wd(workd){}
void *run() {
        for (int i = 0; i < 10; i++) {
            cout<<"thread "<<(long unsigned int)self()<<endl;
            //Currently testing with only work done
            wd++;
            wd.display();
            sleep(1);
            }
        cout<<"thread done "<<(long unsigned int)self()<<endl;
        return NULL;
    }

};
// Driver function
int main(int argc , char ** argv)
{
    shared_integer workdone(0);

    MyThread* thread1 = new MyThread(workdone);
    MyThread* thread2 = new MyThread(workdone);
    MyThread* thread3 = new MyThread(workdone);
    MyThread* thread4 = new MyThread(workdone);

    cout<<"main After creating threads"<<endl;
    thread1->start();
    thread2->start();
    thread3->start();
    thread4->start();
    cout<<"main Before joining first therad"<<endl;
    cout<<"main Before joining second therad"<<endl;
    thread1->join();
    thread2->join();
    thread3->join();
    thread4->join();

    cout<<"main done"<<endl;
return 0;

}

If I would get this code at my job, I would reject it. There is NO valid reason for using the pthread library directly to manage threads. Please use std::thread instead. Normally, I wouldn't even do the effort to read further. The same holds for the mutex, just use std::mutex, std::shared_mutex ... instead.

I see your usage of shared_integer, which raises a few questions:
 - Isn't this std::atomic<int>, less performant? (Yes, I know you don't want to get into it, though, I would still recommend looking into it if you don't expose the mutex)
 - Secondly, your implementation is flawed, as operator-- and display() don't use the lock
 - Thirdly, I would urge you to make this a template. It's easy to make mistakes into this, you only want to have that kind of code once.

Looking at your main-function, it looks like you have a memory leak.

The same code using the C++ standard library:

#include <thread>
#include <iostream>
#include <chrono>
#include <atomic>

int main(int argc , char ** argv)
{
    std::atomic<int> workdonecount{0};

    auto workdone = [&workdonecount]()
    {
        for (int i = 0; i < 10; i++) {
            std::cout<<"thread "<<std::this_thread::get_id()<<std::endl;
            //Currently testing with only work done
            workdonecount++;
            std::cout << "Value became:" << workdonecount << std::endl;
            std::this_thread::sleep_for(std::chrono::seconds{1});
            }
        std::cout<<"thread done "<<std::this_thread::get_id()<<std::endl;
    };

    std::cout<<"main After creating threads"<<std::endl;
    auto thread1 = std::thread(workdone);
    auto thread2 = std::thread(workdone);
    auto thread3 = std::thread(workdone);
    auto thread4 = std::thread(workdone);
    std::cout<<"main Before joining first therad"<<std::endl;
    std::cout<<"main Before joining second therad"<<std::endl;
    thread1.join();
    thread2.join();
    thread3.join();
    thread4.join();

    std::cout<<"main done"<<std::endl;
return 0;

}

Code at compiler explorer using c++17

From C++20, you could even use std::jthread and let the threads join automatically.

PS: This code contains the same bug that you have:

 workdonecount++;
 std::cout << "Value became:" << workdonecount << std::endl;

Should become:

 auto newValue = ++workdonecount;
 std::cout << "Value became: " << newValue << std::endl;