Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Add jobTitle generator #630

Merged
merged 2 commits into from
Feb 23, 2016
Merged

Add jobTitle generator #630

merged 2 commits into from
Feb 23, 2016

Conversation

gregoryduckworth
Copy link
Contributor

To address #626

@@ -8,6 +8,10 @@ class Company extends \Faker\Provider\Base
'{{lastName}} {{companySuffix}}',
);

protected static $jobTitle = array(
'Account Executive', 'Account Manager', 'Accountant', 'Administrative Assistant', 'Assistant Manager', 'Assistant Store Manager', 'Business Analyst', 'Cashier', 'Cook', 'Customer Sales Representative', 'Customer Service', 'Driver', 'Engineer', 'Executive Assistant', 'Financial Analyst', 'Java Developer', 'Maintenance Technician', 'Manager', 'Merchandiser', 'Nurse', 'Occupational Therapist', 'Occupational Therapy Assistant', 'Pharmacist', 'Physcial Therapist', 'Program Manager', 'Project Manager', 'Receptionist', 'Registered Nurse', 'Restaurant Manager', 'Retail Sales Consultant', 'Sales', 'Sales Associate', 'Sales Manager', 'Sales Representative', 'Security Officer', 'Senior Accountant', 'Software Engineer', 'Speech Language Therapist', 'Store Manager', 'Systems Engineer', 'Technician', 'Teller',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this list should be in the en_US locale. For the no-locale formatter, you should rely on the lorem words.

Also, could you please split the line by first letter, to improve code readability?

@gregoryduckworth
Copy link
Contributor Author

@fzaninotto hopefully have implemented your comments correctly.

/*
* @example 'Cashier'
*/
public static function jobTitle()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't be static, because of $this. A unit tests would have showed it to you. This shows me that you haven't tested you own code, it's bad. Please add a unit test.

@gregoryduckworth
Copy link
Contributor Author

Hopefully, fingers crossed, I'm there now.

@fzaninotto
Copy link
Owner

Still missing a unit test

@gregoryduckworth
Copy link
Contributor Author

Added the unit test now.

@hotmeteor
Copy link

Status on this pull?

fzaninotto added a commit that referenced this pull request Feb 23, 2016
@fzaninotto fzaninotto merged commit cae0d90 into fzaninotto:master Feb 23, 2016
@fzaninotto
Copy link
Owner

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants